Throw ArgumentNullException if DSN is null (#2655)

jamescrosswell · web-flow · commit 0aed96926720 · 2023-09-28T15:02:21.000+02:00
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -15,7 +15,7 @@ without native/platform specific bindings and SDKs. See [this ticket for more de
 - Drop .NET 6 Mobile in favor of .NET 7 ([#2624](https://github.com/getsentry/sentry-dotnet/pull/2604))
 
 API Changes:
-
+- If `null` has been supplied as DSN when initializing Sentry, and ArgumentNullException is now thrown ([#2655](https://github.com/getsentry/sentry-dotnet/pull/2655))
 - IHasBreadcrumbs was removed. Use IEventLike instead. ([#2670](https://github.com/getsentry/sentry-dotnet/pull/2670))
 - ISpanContext was removed. Use ITraceContext instead. ([#2668](https://github.com/getsentry/sentry-dotnet/pull/2668))
 - Removed IHasTransactionNameSource. Use ITransactionContext instead. ([#2654](https://github.com/getsentry/sentry-dotnet/pull/2654))
diff --git a/src/Sentry/Internal/SettingLocator.cs b/src/Sentry/Internal/SettingLocator.cs
@@ -35,19 +35,15 @@ public string GetDsn()
         // For DSN only
         // An empty string set on the option should NOT be converted to null because it is used
         // to indicate the the SDK is disabled.
-
-        var dsn = _options.Dsn;
-        if (dsn != null)
+        _options.Dsn ??= GetEnvironmentVariable(Constants.DsnEnvironmentVariable)
+                         ?? AssemblyForAttributes?.GetCustomAttribute<DsnAttribute>()?.Dsn;
+        if (_options.Dsn is null)
         {
-            return dsn;
+            throw new ArgumentNullException("You must supply a DSN to use Sentry." +
+                                            "To disable Sentry, pass an empty string: \"\"." +
+                                            "See https://docs.sentry.io/platforms/dotnet/configuration/options/#dsn");
         }
-
-        dsn = GetEnvironmentVariable(Constants.DsnEnvironmentVariable)
-              ?? AssemblyForAttributes?.GetCustomAttribute<DsnAttribute>()?.Dsn
-              ?? Sentry.Constants.DisableSdkDsnValue;
-
-        _options.Dsn = dsn;
-        return dsn;
+        return _options.Dsn;
     }
 
     public string GetEnvironment() => GetEnvironment(true)!;
diff --git a/src/Sentry/SentrySdk.cs b/src/Sentry/SentrySdk.cs
@@ -40,7 +40,7 @@ internal static IHub InitHub(SentryOptions options)
         // from anywhere else, return a disabled hub.
         if (Dsn.IsDisabled(dsnString))
         {
-            options.LogWarning("Init was called but no DSN was provided nor located. Sentry SDK will be disabled.");
+            options.LogWarning("Init called with an empty string as the DSN. Sentry SDK will be disabled.");
             return DisabledHub.Instance;
         }
 
diff --git a/test/Sentry.AspNetCore.Tests/AspNetCoreSentryWebHostBuilder.IntegrationTests.cs b/test/Sentry.AspNetCore.Tests/AspNetCoreSentryWebHostBuilder.IntegrationTests.cs
@@ -27,11 +27,9 @@ public void UseSentry_ValidDsnString_EnablesSdk()
     }
 
     [Fact]
-    public void UseSentry_NoDsnProvided_DisabledSdk()
+    public void UseSentry_NoDsnProvided_ThrowsException()
     {
-        _ = _webHostBuilder.UseSentry().Build();
-
-        Assert.False(SentrySdk.IsEnabled);
+        Assert.Throws<ArgumentNullException>(() => _webHostBuilder.UseSentry().Build());
     }
 
     [Fact]
diff --git a/test/Sentry.Extensions.Logging.Tests/SentryLoggerFactoryExtensionsTests.cs b/test/Sentry.Extensions.Logging.Tests/SentryLoggerFactoryExtensionsTests.cs
@@ -64,6 +64,7 @@ public void AddSentry_NoDiagnosticSet_MelSet()
         var sut = Substitute.For<ILoggerFactory>();
         _ = sut.AddSentry(o =>
         {
+            o.Dsn = Sentry.Constants.DisableSdkDsnValue;
             o.Debug = true;
             options = o;
         });
@@ -79,6 +80,7 @@ public void AddSentry_DiagnosticSet_NoOverriden()
         var diagnosticLogger = Substitute.For<IDiagnosticLogger>();
         _ = sut.AddSentry(o =>
         {
+            o.Dsn = Sentry.Constants.DisableSdkDsnValue;
             o.Debug = true;
             Assert.Null(o.DiagnosticLogger);
             o.DiagnosticLogger = diagnosticLogger;
@@ -93,7 +95,11 @@ public void AddSentry_WithOptionsCallback_CallbackInvoked()
     {
         var callbackInvoked = false;
         var expected = Substitute.For<ILoggerFactory>();
-        _ = expected.AddSentry(_ => callbackInvoked = true);
+        _ = expected.AddSentry(o =>
+        {
+            o.Dsn = Sentry.Constants.DisableSdkDsnValue;
+            callbackInvoked = true;
+        });
 
         Assert.True(callbackInvoked);
     }
@@ -112,7 +118,7 @@ public void AddSentry_NoOptionsDelegate_ProviderAdded()
     public void AddSentry_ReturnsSameFactory()
     {
         var expected = Substitute.For<ILoggerFactory>();
-        var actual = expected.AddSentry();
+        var actual = expected.AddSentry(o => o.Dsn = Sentry.Constants.DisableSdkDsnValue);
 
         Assert.Same(expected, actual);
     }
@@ -121,7 +127,7 @@ public void AddSentry_ReturnsSameFactory()
     public void AddSentry_ConfigureOptionsOverload_ReturnsSameFactory()
     {
         var expected = Substitute.For<ILoggerFactory>();
-        var actual = expected.AddSentry(_ => { });
+        var actual = expected.AddSentry(o => o.Dsn = Sentry.Constants.DisableSdkDsnValue);
 
         Assert.Same(expected, actual);
     }
@@ -134,6 +140,7 @@ public void AddSentry_ConfigureOptionsOverload_InvokesCallback()
         var invoked = false;
         _ = expected.AddSentry(o =>
         {
+            o.Dsn = Sentry.Constants.DisableSdkDsnValue;
             Assert.NotNull(o);
             invoked = true;
         });
diff --git a/test/Sentry.NLog.Tests/SentryTargetTests.cs b/test/Sentry.NLog.Tests/SentryTargetTests.cs
@@ -454,7 +454,7 @@ void Continuation(Exception _)
     [Fact]
     public void InitializeTarget_InitializesSdk()
     {
-        _fixture.Options.Dsn = null;
+        _fixture.Options.Dsn = Sentry.Constants.DisableSdkDsnValue;
         _fixture.SdkDisposeHandle = null;
         _fixture.Options.InitializeSdk = true;
 
@@ -468,7 +468,7 @@ public void InitializeTarget_InitializesSdk()
             _fixture.GetLoggerFactory();
 
             var logOutput = logWriter.ToString();
-            Assert.Contains("Init was called but no DSN was provided nor located. Sentry SDK will be disabled.", logOutput);
+            Assert.Contains("Init called with an empty string as the DSN. Sentry SDK will be disabled.", logOutput);
         }
         finally
         {
diff --git a/test/Sentry.Serilog.Tests/SerilogAspNetSentrySdkTestFixture.cs b/test/Sentry.Serilog.Tests/SerilogAspNetSentrySdkTestFixture.cs
@@ -28,7 +28,7 @@ protected override void ConfigureBuilder(WebHostBuilder builder)
         builder.ConfigureLogging(loggingBuilder =>
         {
             var logger = new LoggerConfiguration()
-                .WriteTo.Sentry()
+                .WriteTo.Sentry(ValidDsn)
                 .CreateLogger();
             loggingBuilder.AddSerilog(logger);
         });
diff --git a/test/Sentry.Tests/Internals/SettingLocatorTests.cs b/test/Sentry.Tests/Internals/SettingLocatorTests.cs
@@ -153,14 +153,11 @@ public void GetDsn_WithOptionAlreadySet_IgnoresAttribute()
     }
 
     [Fact]
-    public void GetDsn_WithNoValueAnywhere_ReturnsAndSetsDisabledDsn()
+    public void GetDsn_WithNoValueAnywhere_ThrowsException()
     {
         var options = new SentryOptions();
 
-        var dsn = options.SettingLocator.GetDsn();
-
-        Assert.Equal(DisableSdkDsnValue, dsn);
-        Assert.Equal(DisableSdkDsnValue, options.Dsn);
+        Assert.Throws<ArgumentNullException>(() => options.SettingLocator.GetDsn());
     }
 
     [Fact]
diff --git a/test/Sentry.Tests/SentrySdkTests.cs b/test/Sentry.Tests/SentrySdkTests.cs
@@ -141,14 +141,15 @@ public void Init_EmptyDsn_LogsWarning()
     {
         var options = new SentryOptions
         {
+            Dsn = Constants.DisableSdkDsnValue,
             DiagnosticLogger = _logger,
             Debug = true,
             InitNativeSdks = false
         };
 
         using (SentrySdk.Init(options))
         {
-            _logger.Received(1).Log(SentryLevel.Warning, "Init was called but no DSN was provided nor located. Sentry SDK will be disabled.");
+            _logger.Received(1).Log(SentryLevel.Warning, "Init called with an empty string as the DSN. Sentry SDK will be disabled.");
         }
     }
 
@@ -176,6 +177,7 @@ public void Init_EmptyDsnDisabledDiagnostics_DoesNotLogWarning()
     {
         var options = new SentryOptions
         {
+            Dsn = Constants.DisableSdkDsnValue,
             DiagnosticLogger = _logger,
             Debug = false,
             InitNativeSdks = false,
@@ -821,14 +823,14 @@ public void Implements_ScopeManagement()
     [Fact]
     public void InitHub_NoDsn_DisposeDoesNotThrow()
     {
-        var sut = SentrySdk.InitHub(new SentryOptions()) as IDisposable;
+        var sut = SentrySdk.InitHub(new SentryOptions(){Dsn = Constants.DisableSdkDsnValue}) as IDisposable;
         sut?.Dispose();
     }
 
     [Fact]
     public async Task InitHub_NoDsn_FlushAsyncDoesNotThrow()
     {
-        var sut = SentrySdk.InitHub(new SentryOptions());
+        var sut = SentrySdk.InitHub(new SentryOptions(){Dsn = Constants.DisableSdkDsnValue});
         await sut.FlushAsync();
     }
 

Original file line number	Diff line number	Diff line change
`@@ -40,7 +40,7 @@ internal static IHub InitHub(SentryOptions options)`
`40`	`40`	`// from anywhere else, return a disabled hub.`
`41`	`41`	`if (Dsn.IsDisabled(dsnString))`
`42`	`42`	`{`
`43`		`- options.LogWarning("Init was called but no DSN was provided nor located. Sentry SDK will be disabled.");`
	`43`	`+ options.LogWarning("Init called with an empty string as the DSN. Sentry SDK will be disabled.");`
`44`	`44`	`return DisabledHub.Instance;`
`45`	`45`	`}`
`46`	`46`
Original file line number	Diff line number	Diff line change
`@@ -27,11 +27,9 @@ public void UseSentry_ValidDsnString_EnablesSdk()`
`27`	`27`	`}`
`28`	`28`
`29`	`29`	`[Fact]`
`30`		`- public void UseSentry_NoDsnProvided_DisabledSdk()`
	`30`	`+ public void UseSentry_NoDsnProvided_ThrowsException()`
`31`	`31`	`{`
`32`		`- _ = _webHostBuilder.UseSentry().Build();`
`33`		`-`
`34`		`- Assert.False(SentrySdk.IsEnabled);`
	`32`	`+ Assert.Throws<ArgumentNullException>(() => _webHostBuilder.UseSentry().Build());`
`35`	`33`	`}`
`36`	`34`
`37`	`35`	`[Fact]`
Original file line number	Diff line number	Diff line change
`@@ -64,6 +64,7 @@ public void AddSentry_NoDiagnosticSet_MelSet()`
`64`	`64`	`var sut = Substitute.For<ILoggerFactory>();`
`65`	`65`	`_ = sut.AddSentry(o =>`
`66`	`66`	`{`
	`67`	`+ o.Dsn = Sentry.Constants.DisableSdkDsnValue;`
`67`	`68`	`o.Debug = true;`
`68`	`69`	`options = o;`
`69`	`70`	`});`
`@@ -79,6 +80,7 @@ public void AddSentry_DiagnosticSet_NoOverriden()`
`79`	`80`	`var diagnosticLogger = Substitute.For<IDiagnosticLogger>();`
`80`	`81`	`_ = sut.AddSentry(o =>`
`81`	`82`	`{`
	`83`	`+ o.Dsn = Sentry.Constants.DisableSdkDsnValue;`
`82`	`84`	`o.Debug = true;`
`83`	`85`	`Assert.Null(o.DiagnosticLogger);`
`84`	`86`	`o.DiagnosticLogger = diagnosticLogger;`
`@@ -93,7 +95,11 @@ public void AddSentry_WithOptionsCallback_CallbackInvoked()`
`93`	`95`	`{`
`94`	`96`	`var callbackInvoked = false;`
`95`	`97`	`var expected = Substitute.For<ILoggerFactory>();`
`96`		`- _ = expected.AddSentry(_ => callbackInvoked = true);`
	`98`	`+ _ = expected.AddSentry(o =>`
	`99`	`+ {`
	`100`	`+ o.Dsn = Sentry.Constants.DisableSdkDsnValue;`
	`101`	`+ callbackInvoked = true;`
	`102`	`+ });`
`97`	`103`
`98`	`104`	`Assert.True(callbackInvoked);`
`99`	`105`	`}`
`@@ -112,7 +118,7 @@ public void AddSentry_NoOptionsDelegate_ProviderAdded()`
`112`	`118`	`public void AddSentry_ReturnsSameFactory()`
`113`	`119`	`{`
`114`	`120`	`var expected = Substitute.For<ILoggerFactory>();`
`115`		`- var actual = expected.AddSentry();`
	`121`	`+ var actual = expected.AddSentry(o => o.Dsn = Sentry.Constants.DisableSdkDsnValue);`
`116`	`122`
`117`	`123`	`Assert.Same(expected, actual);`
`118`	`124`	`}`
`@@ -121,7 +127,7 @@ public void AddSentry_ReturnsSameFactory()`
`121`	`127`	`public void AddSentry_ConfigureOptionsOverload_ReturnsSameFactory()`
`122`	`128`	`{`
`123`	`129`	`var expected = Substitute.For<ILoggerFactory>();`
`124`		`- var actual = expected.AddSentry(_ => { });`
	`130`	`+ var actual = expected.AddSentry(o => o.Dsn = Sentry.Constants.DisableSdkDsnValue);`
`125`	`131`
`126`	`132`	`Assert.Same(expected, actual);`
`127`	`133`	`}`
`@@ -134,6 +140,7 @@ public void AddSentry_ConfigureOptionsOverload_InvokesCallback()`
`134`	`140`	`var invoked = false;`
`135`	`141`	`_ = expected.AddSentry(o =>`
`136`	`142`	`{`
	`143`	`+ o.Dsn = Sentry.Constants.DisableSdkDsnValue;`
`137`	`144`	`Assert.NotNull(o);`
`138`	`145`	`invoked = true;`
`139`	`146`	`});`
Original file line number	Diff line number	Diff line change
`@@ -454,7 +454,7 @@ void Continuation(Exception _)`
`454`	`454`	`[Fact]`
`455`	`455`	`public void InitializeTarget_InitializesSdk()`
`456`	`456`	`{`
`457`		`- _fixture.Options.Dsn = null;`
	`457`	`+ _fixture.Options.Dsn = Sentry.Constants.DisableSdkDsnValue;`
`458`	`458`	`_fixture.SdkDisposeHandle = null;`
`459`	`459`	`_fixture.Options.InitializeSdk = true;`
`460`	`460`
`@@ -468,7 +468,7 @@ public void InitializeTarget_InitializesSdk()`
`468`	`468`	`_fixture.GetLoggerFactory();`
`469`	`469`
`470`	`470`	`var logOutput = logWriter.ToString();`
`471`		`- Assert.Contains("Init was called but no DSN was provided nor located. Sentry SDK will be disabled.", logOutput);`
	`471`	`+ Assert.Contains("Init called with an empty string as the DSN. Sentry SDK will be disabled.", logOutput);`
`472`	`472`	`}`
`473`	`473`	`finally`
`474`	`474`	`{`
Original file line number	Diff line number	Diff line change
`@@ -28,7 +28,7 @@ protected override void ConfigureBuilder(WebHostBuilder builder)`
`28`	`28`	`builder.ConfigureLogging(loggingBuilder =>`
`29`	`29`	`{`
`30`	`30`	`var logger = new LoggerConfiguration()`
`31`		`- .WriteTo.Sentry()`
	`31`	`+ .WriteTo.Sentry(ValidDsn)`
`32`	`32`	`.CreateLogger();`
`33`	`33`	`loggingBuilder.AddSerilog(logger);`
`34`	`34`	`});`
Original file line number	Diff line number	Diff line change
`@@ -153,14 +153,11 @@ public void GetDsn_WithOptionAlreadySet_IgnoresAttribute()`
`153`	`153`	`}`
`154`	`154`
`155`	`155`	`[Fact]`
`156`		`- public void GetDsn_WithNoValueAnywhere_ReturnsAndSetsDisabledDsn()`
	`156`	`+ public void GetDsn_WithNoValueAnywhere_ThrowsException()`
`157`	`157`	`{`
`158`	`158`	`var options = new SentryOptions();`
`159`	`159`
`160`		`- var dsn = options.SettingLocator.GetDsn();`
`161`		`-`
`162`		`- Assert.Equal(DisableSdkDsnValue, dsn);`
`163`		`- Assert.Equal(DisableSdkDsnValue, options.Dsn);`
	`160`	`+ Assert.Throws<ArgumentNullException>(() => options.SettingLocator.GetDsn());`
`164`	`161`	`}`
`165`	`162`
`166`	`163`	`[Fact]`
Original file line number	Diff line number	Diff line change
`@@ -141,14 +141,15 @@ public void Init_EmptyDsn_LogsWarning()`
`141`	`141`	`{`
`142`	`142`	`var options = new SentryOptions`
`143`	`143`	`{`
	`144`	`+ Dsn = Constants.DisableSdkDsnValue,`
`144`	`145`	`DiagnosticLogger = _logger,`
`145`	`146`	`Debug = true,`
`146`	`147`	`InitNativeSdks = false`
`147`	`148`	`};`
`148`	`149`
`149`	`150`	`using (SentrySdk.Init(options))`
`150`	`151`	`{`
`151`		`- _logger.Received(1).Log(SentryLevel.Warning, "Init was called but no DSN was provided nor located. Sentry SDK will be disabled.");`
	`152`	`+ _logger.Received(1).Log(SentryLevel.Warning, "Init called with an empty string as the DSN. Sentry SDK will be disabled.");`
`152`	`153`	`}`
`153`	`154`	`}`
`154`	`155`
`@@ -176,6 +177,7 @@ public void Init_EmptyDsnDisabledDiagnostics_DoesNotLogWarning()`
`176`	`177`	`{`
`177`	`178`	`var options = new SentryOptions`
`178`	`179`	`{`
	`180`	`+ Dsn = Constants.DisableSdkDsnValue,`
`179`	`181`	`DiagnosticLogger = _logger,`
`180`	`182`	`Debug = false,`
`181`	`183`	`InitNativeSdks = false,`
`@@ -821,14 +823,14 @@ public void Implements_ScopeManagement()`
`821`	`823`	`[Fact]`
`822`	`824`	`public void InitHub_NoDsn_DisposeDoesNotThrow()`
`823`	`825`	`{`
`824`		`- var sut = SentrySdk.InitHub(new SentryOptions()) as IDisposable;`
	`826`	`+ var sut = SentrySdk.InitHub(new SentryOptions(){Dsn = Constants.DisableSdkDsnValue}) as IDisposable;`
`825`	`827`	`sut?.Dispose();`
`826`	`828`	`}`
`827`	`829`
`828`	`830`	`[Fact]`
`829`	`831`	`public async Task InitHub_NoDsn_FlushAsyncDoesNotThrow()`
`830`	`832`	`{`
`831`		`- var sut = SentrySdk.InitHub(new SentryOptions());`
	`833`	`+ var sut = SentrySdk.InitHub(new SentryOptions(){Dsn = Constants.DisableSdkDsnValue});`
`832`	`834`	`await sut.FlushAsync();`
`833`	`835`	`}`
`834`	`836`