enh: cache parsed dsn (#2365)

* add cached dsn * update * update * update * update * update CHANGELOG * update * re-evaluate when dsn is set * update comment
getsentry · Oct 22, 2024 · 8f95e33 · 8f95e33
1 parent 136c365
commit 8f95e33
Show file tree

Hide file tree

Showing 8 changed files with 75 additions and 12 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -1,5 +1,11 @@
 # Changelog
 
+## Unreleased
+
+### Enhancements
+
+- Cache parsed DSN ([#2365](https://github.com/getsentry/sentry-dart/pull/2365))
+
 ## 8.10.0-beta.2
 
 ### Fixes

diff --git a/dart/lib/src/sentry.dart b/dart/lib/src/sentry.dart
@@ -273,7 +273,7 @@ class Sentry {
     }
 
     // try parsing the dsn
-    Dsn.parse(options.dsn!);
+    options.parsedDsn;
 
     return true;
   }

diff --git a/dart/lib/src/sentry_baggage.dart b/dart/lib/src/sentry_baggage.dart
@@ -95,9 +95,7 @@ class SentryBaggage {
   setValuesFromScope(Scope scope, SentryOptions options) {
     final propagationContext = scope.propagationContext;
     setTraceId(propagationContext.traceId.toString());
-    if (options.dsn != null) {
-      setPublicKey(Dsn.parse(options.dsn!).publicKey);
-    }
+    setPublicKey(options.parsedDsn.publicKey);
     if (options.release != null) {
       setRelease(options.release!);
     }

diff --git a/dart/lib/src/sentry_options.dart b/dart/lib/src/sentry_options.dart
@@ -23,9 +23,34 @@ class SentryOptions {
   /// Default Log level if not specified Default is DEBUG
   static final SentryLevel _defaultDiagnosticLevel = SentryLevel.debug;
 
-  /// The DSN tells the SDK where to send the events to. If an empty string is
-  /// used, the SDK will not send any events.
-  String? dsn;
+  String? _dsn;
+  Dsn? _parsedDsn;
+
+  /// The DSN tells the SDK where to send the events to.
+  /// If an empty string is used, the SDK will not send any events.
+  String? get dsn => _dsn;
+
+  set dsn(String? value) {
+    if (_dsn != value) {
+      _dsn = value;
+      _parsedDsn = null; // Invalidate the cached parsed DSN
+    }
+  }
+
+  /// Evaluates and parses the DSN.
+  /// May throw an exception if the DSN is invalid.
+  @internal
+  Dsn get parsedDsn {
+    _parsedDsn ??= _parseDsn();
+    return _parsedDsn!;
+  }
+
+  Dsn _parseDsn() {
+    if (_dsn == null || _dsn!.isEmpty) {
+      throw StateError('DSN is null or empty');
+    }
+    return Dsn.parse(_dsn!);
+  }
 
   /// If [compressPayload] is `true` the outgoing HTTP payloads are compressed
   /// using gzip. Otherwise, the payloads are sent in plain UTF8-encoded JSON
@@ -525,7 +550,8 @@ class SentryOptions {
   /// iOS only supports http proxies, while macOS also supports socks.
   SentryProxy? proxy;
 
-  SentryOptions({this.dsn, PlatformChecker? checker}) {
+  SentryOptions({String? dsn, PlatformChecker? checker}) {
+    this.dsn = dsn;
     if (checker != null) {
       platformChecker = checker;
     }

diff --git a/dart/lib/src/sentry_tracer.dart b/dart/lib/src/sentry_tracer.dart
@@ -377,7 +377,7 @@ class SentryTracer extends ISentrySpan {
 
     _sentryTraceContextHeader = SentryTraceContextHeader(
       _rootSpan.context.traceId,
-      Dsn.parse(_hub.options.dsn!).publicKey,
+      _hub.options.parsedDsn.publicKey,
       release: _hub.options.release,
       environment: _hub.options.environment,
       userId: null, // because of PII not sending it for now

diff --git a/dart/lib/src/transport/http_transport.dart b/dart/lib/src/transport/http_transport.dart
@@ -30,8 +30,8 @@ class HttpTransport implements Transport {
   }
 
   HttpTransport._(this._options, this._rateLimiter)
-      : _requestHandler = HttpTransportRequestHandler(
-            _options, Dsn.parse(_options.dsn!).postUri);
+      : _requestHandler =
+            HttpTransportRequestHandler(_options, _options.parsedDsn.postUri);
 
   @override
   Future<SentryId?> send(SentryEnvelope envelope) async {

diff --git a/dart/lib/src/transport/http_transport_request_handler.dart b/dart/lib/src/transport/http_transport_request_handler.dart
@@ -17,7 +17,7 @@ class HttpTransportRequestHandler {
   late _CredentialBuilder _credentialBuilder;
 
   HttpTransportRequestHandler(this._options, this._requestUri)
-      : _dsn = Dsn.parse(_options.dsn!),
+      : _dsn = _options.parsedDsn,
         _headers = _buildHeaders(
           _options.platformChecker.isWeb,
           _options.sentryClientName,

diff --git a/dart/test/sentry_options_test.dart b/dart/test/sentry_options_test.dart
@@ -198,4 +198,37 @@ void main() {
 
     expect(options.enableDartSymbolication, true);
   });
+
+  test('parsedDsn is correctly parsed and cached', () {
+    final options = defaultTestOptions();
+
+    // Access parsedDsn for the first time
+    final parsedDsn1 = options.parsedDsn;
+
+    // Access parsedDsn again
+    final parsedDsn2 = options.parsedDsn;
+
+    // Should return the same instance since it's cached
+    expect(identical(parsedDsn1, parsedDsn2), isTrue);
+
+    // Verify the parsed DSN fields
+    final manuallyParsedDsn = Dsn.parse(options.dsn!);
+    expect(parsedDsn1.publicKey, manuallyParsedDsn.publicKey);
+    expect(parsedDsn1.postUri, manuallyParsedDsn.postUri);
+    expect(parsedDsn1.secretKey, manuallyParsedDsn.secretKey);
+    expect(parsedDsn1.projectId, manuallyParsedDsn.projectId);
+    expect(parsedDsn1.uri, manuallyParsedDsn.uri);
+  });
+
+  test('parsedDsn throws when DSN is null', () {
+    final options = defaultTestOptions()..dsn = null;
+
+    expect(() => options.parsedDsn, throwsA(isA<StateError>()));
+  });
+
+  test('parsedDsn throws when DSN is empty', () {
+    final options = defaultTestOptions()..dsn = '';
+
+    expect(() => options.parsedDsn, throwsA(isA<StateError>()));
+  });
 }