fix: Allow a DSN string.Empty to be overwritten by the environment (#…

…3147)
getsentry · Feb 14, 2024 · 1aad9ab · 1aad9ab
1 parent a3d4c4f
commit 1aad9ab
Show file tree

Hide file tree

Showing 3 changed files with 82 additions and 11 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -2,6 +2,10 @@
 
 ## Unreleased
 
+### Fixes
+
+- The SDK can be disabled by setting `options.Dsn = "";` By convention, the SDK allows the DSN set to `string.Empty` to be overwritten by the environment. ([#3147](https://github.com/getsentry/sentry-dotnet/pull/3147))
+
 ### Dependencies
 
 - Bump CLI from v2.28.0 to v2.28.6 ([#3145](https://github.com/getsentry/sentry-dotnet/pull/3145), [#3148](https://github.com/getsentry/sentry-dotnet/pull/3148))

diff --git a/src/Sentry/Internal/SettingLocator.cs b/src/Sentry/Internal/SettingLocator.cs
@@ -33,17 +33,33 @@ public SettingLocator(SentryOptions options)
     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.
-        _options.Dsn ??= GetEnvironmentVariable(Constants.DsnEnvironmentVariable)
-                         ?? AssemblyForAttributes?.GetCustomAttribute<DsnAttribute>()?.Dsn;
-        if (_options.Dsn is null)
+
+        if (!string.IsNullOrEmpty(_options.Dsn))
+        {
+            return _options.Dsn;
+        }
+
+        var dsn = GetEnvironmentVariable(Constants.DsnEnvironmentVariable)
+                  ?? AssemblyForAttributes?.GetCustomAttribute<DsnAttribute>()?.Dsn;
+
+        // If there has been no DSN provided (`null`) and none has been found in the environment we consider this a
+        // failed configuration and throw
+        // By conventions, skip this if the DSN is not `null` i.e. `string.Empty`
+        if (_options.Dsn is null && dsn is null)
         {
             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");
         }
-        return _options.Dsn;
+
+        // Overwriting the `string.Empty` with the DSN found in the environment
+        if (dsn is not null)
+        {
+            _options.Dsn = dsn;
+        }
+
+        Debug.Assert(_options.Dsn != null, "Dsn can't be null at this point based on the rules above");
+        return _options.Dsn!;
     }
 
     public string GetEnvironment() => GetEnvironment(true)!;

diff --git a/test/Sentry.Tests/Internals/SettingLocatorTests.cs b/test/Sentry.Tests/Internals/SettingLocatorTests.cs
@@ -5,9 +5,9 @@ namespace Sentry.Tests.Internals;
 public class SettingLocatorTests
 {
     private const string DisableSdkDsnValue = SentryConstants.DisableSdkDsnValue;
-    private const string DsnEnvironmentVariable = Internal.Constants.DsnEnvironmentVariable;
-    private const string EnvironmentEnvironmentVariable = Internal.Constants.EnvironmentEnvironmentVariable;
-    private const string ReleaseEnvironmentVariable = Internal.Constants.ReleaseEnvironmentVariable;
+    private const string DsnEnvironmentVariable = Constants.DsnEnvironmentVariable;
+    private const string EnvironmentEnvironmentVariable = Constants.EnvironmentEnvironmentVariable;
+    private const string ReleaseEnvironmentVariable = Constants.ReleaseEnvironmentVariable;
 
     private static Assembly GetAssemblyWithDsn(string dsn) =>
         AssemblyCreationHelper.CreateAssemblyWithDsnAttribute(dsn);
@@ -82,6 +82,17 @@ public void CanUseOtherAssembly()
         Assert.Same(expected, actual);
     }
 
+    [Fact]
+    public void GetDsn_WithEmptyString_DoesNotThrow()
+    {
+        var options = new SentryOptions { Dsn = DisableSdkDsnValue };
+
+        var dsn = options.SettingLocator.GetDsn();
+
+        Assert.Equal(DisableSdkDsnValue, dsn);
+        Assert.Equal(DisableSdkDsnValue, options.Dsn);
+    }
+
     [Fact]
     public void GetDsn_WithDsnInEnvironmentVariable_ReturnsAndSetsDsn()
     {
@@ -123,7 +134,7 @@ public void GetDsn_WithDsnInBothEnvironmentVariableAndAttribute_ReturnsAndSetsDs
     }
 
     [Fact]
-    public void GetDsn_WithOptionAlreadySet_IgnoresEnvironmentVariable()
+    public void GetDsn_DsnIsNonEmptyString_IgnoresEnvironmentVariable()
     {
         const string validDsn1 = ValidDsn + "1";
         const string validDsn2 = ValidDsn + "2";
@@ -138,7 +149,7 @@ public void GetDsn_WithOptionAlreadySet_IgnoresEnvironmentVariable()
     }
 
     [Fact]
-    public void GetDsn_WithOptionAlreadySet_IgnoresAttribute()
+    public void GetDsn_DsnIsNonEmptyString_IgnoresAttribute()
     {
         const string validDsn1 = ValidDsn + "1";
         const string validDsn2 = ValidDsn + "2";
@@ -197,6 +208,46 @@ public void GetDsn_WithDisabledDsnInEnvironmentVariableButValidDsnInAttribute_Re
         Assert.Equal(DisableSdkDsnValue, options.Dsn);
     }
 
+    [Fact]
+    public void GetDsn_DsnIsStringEmptyButEnvironmentValid_ReturnsAndSetsEnvironmentDsn()
+    {
+        var options = new SentryOptions { Dsn = string.Empty };
+        options.FakeSettings().EnvironmentVariables[DsnEnvironmentVariable] = ValidDsn;
+
+        var dsn = options.SettingLocator.GetDsn();
+
+        Assert.Equal(ValidDsn, dsn);
+        Assert.Equal(ValidDsn, options.Dsn);
+    }
+
+    [Fact]
+    public void GetDsn_DsnIsStringEmptyButAttributeValid_ReturnsAndSetsAttributeDsn()
+    {
+        var options = new SentryOptions { Dsn = string.Empty };
+        options.FakeSettings().AssemblyForAttributes = GetAssemblyWithDsn(ValidDsn);
+
+        var dsn = options.SettingLocator.GetDsn();
+
+        Assert.Equal(ValidDsn, dsn);
+        Assert.Equal(ValidDsn, options.Dsn);
+    }
+
+    [Fact]
+    public void GetDsn_DsnIsStringEmptyButEnvironmentAndAttributeValid_ReturnsAndSetsEnvironmentDsn()
+    {
+        const string validDsn1 = ValidDsn + "1";
+        const string validDsn2 = ValidDsn + "2";
+
+        var options = new SentryOptions { Dsn = string.Empty };
+        options.FakeSettings().EnvironmentVariables[DsnEnvironmentVariable] = validDsn1;
+        options.FakeSettings().AssemblyForAttributes = GetAssemblyWithDsn(validDsn2);
+
+        var dsn = options.SettingLocator.GetDsn();
+
+        Assert.Equal(validDsn1, dsn);
+        Assert.Equal(validDsn1, options.Dsn);
+    }
+
     [Fact]
     public void GetEnvironment_WithEnvironmentInEnvironmentVariable_ReturnsAndSetsEnvironment()
     {