ref: Unified API code review

dart/example_web/pubspec.yaml

@@ -1,7 +1,5 @@
 name: sentry_dart_web_example
 description: An absolute bare-bones web app.
-# version: 1.0.0
-#homepage: https://www.example.com
 
 environment:
   sdk: ^2.0.0

dart/lib/src/hub.dart

@@ -249,7 +249,7 @@ class Hub {
   /// Clones the Hub
   Hub clone() {
     if (!_isEnabled) {
-      _options..logger(SentryLevel.warning, 'Disabled Hub cloned.');
+      _options.logger(SentryLevel.warning, 'Disabled Hub cloned.');
     }
     final clone = Hub(_options);
     for (final item in _stack) {

dart/lib/src/scope.dart

@@ -75,8 +75,8 @@ class Scope {
     }
 
     // run before breadcrumb callback if set
-    if (_options.beforeBreadcrumbCallback != null) {
-      breadcrumb = _options.beforeBreadcrumbCallback(breadcrumb, hint);
+    if (_options.beforeBreadcrumb != null) {
+      breadcrumb = _options.beforeBreadcrumb(breadcrumb, hint);
 
       if (breadcrumb == null) {
         _options.logger(

dart/lib/src/sentry_client.dart

@@ -7,8 +7,8 @@ import 'sentry_client_stub.dart'
     if (dart.library.html) 'sentry_browser_client.dart'
     if (dart.library.io) 'sentry_io_client.dart';
 import 'sentry_options.dart';
+import 'transport/http_transport.dart';
 import 'transport/noop_transport.dart';
-import 'transport/transport.dart';
 import 'version.dart';
 
 /// Logs crash reports and events to the Sentry.io service.
@@ -22,27 +22,27 @@ abstract class SentryClient {
   SentryClient.base(this._options, {String origin}) {
     _random = _options.sampleRate == null ? null : Random();
     if (_options.transport is NoOpTransport) {
-      _options.transport = Transport(options: _options, origin: origin);
+      _options.transport = HttpTransport(options: _options, origin: origin);
     }
   }
 
   SentryOptions _options;
 
   Random _random;
 
+  static final _sentryId = Future.value(SentryId.empty());
+
   /// Reports an [event] to Sentry.io.
   Future<SentryId> captureEvent(
     SentryEvent event, {
     Scope scope,
     dynamic hint,
   }) async {
-    final emptyFuture = Future.value(SentryId.empty());
-
     event = _processEvent(event, eventProcessors: _options.eventProcessors);
 
     // dropped by sampling or event processors
     if (event == null) {
-      return emptyFuture;
+      return _sentryId;
     }
 
     if (scope != null) {
@@ -53,14 +53,14 @@ abstract class SentryClient {
 
     // dropped by scope event processors
     if (event == null) {
-      return emptyFuture;
+      return _sentryId;
     }
 
     event = _prepareEvent(event);
 
-    if (_options.beforeSendCallback != null) {
+    if (_options.beforeSend != null) {
       try {
-        event = _options.beforeSendCallback(event, hint);
+        event = _options.beforeSend(event, hint);
       } catch (err) {
         _options.logger(
           SentryLevel.error,
@@ -69,7 +69,7 @@ abstract class SentryClient {
       }
       if (event == null) {
         _options.logger(SentryLevel.debug, 'Event was dropped by a processor');
-        return emptyFuture;
+        return _sentryId;
       }
     }
 

dart/lib/src/sentry_options.dart

@@ -92,11 +92,11 @@ class SentryOptions {
 
   /// This function is called with an SDK specific event object and can return a modified event
   /// object or nothing to skip reporting the event
-  BeforeSendCallback beforeSendCallback;
+  BeforeSendCallback beforeSend;
 
   /// This function is called with an SDK specific breadcrumb object before the breadcrumb is added
   /// to the scope. When nothing is returned from the function, the breadcrumb is dropped
-  BeforeBreadcrumbCallback beforeBreadcrumbCallback;
+  BeforeBreadcrumbCallback beforeBreadcrumb;
 
   /// Sets the release. SDK will try to automatically configure a release out of the box
   String release;
@@ -131,8 +131,6 @@ class SentryOptions {
   set transport(Transport transport) =>
       _transport = transport ?? NoOpTransport();
 
-  // TODO: transportGate, connectionTimeoutMillis, readTimeoutMillis, hostnameVerifier, sslSocketFactory, proxy
-
   /// Sets the distribution. Think about it together with release and environment
   String dist;
 

dart/lib/src/transport/http_transport.dart

@@ -0,0 +1,138 @@
+import 'dart:async';
+import 'dart:convert';
+
+import 'package:meta/meta.dart';
+
+import '../protocol.dart';
+import '../sentry_options.dart';
+import '../utils.dart';
+import 'noop_encode.dart' if (dart.library.io) 'encode.dart';
+import 'transport.dart';
+
+/// A transport is in charge of sending the event to the Sentry server.
+class HttpTransport implements Transport {
+  final SentryOptions _options;
+
+  @visibleForTesting
+  final Dsn dsn;
+
+  /// Use for browser stacktrace
+  final String _origin;
+
+  CredentialBuilder _credentialBuilder;
+
+  final Map<String, String> _headers;
+
+  HttpTransport({@required SentryOptions options, String origin})
+      : _options = options,
+        _origin = origin,
+        dsn = Dsn.parse(options.dsn),
+        _headers = _buildHeaders(sdkIdentifier: options.sdk.identifier) {
+    _credentialBuilder = CredentialBuilder(
+      dsn: Dsn.parse(options.dsn),
+      clientId: options.sdk.identifier,
+      clock: options.clock,
+    );
+  }
+
+  @override
+  Future<SentryId> send(SentryEvent event) async {
+    final data = event.toJson(origin: _origin);
+
+    final body = _bodyEncoder(
+      data,
+      _headers,
+      compressPayload: _options.compressPayload,
+    );
+
+    final response = await _options.httpClient.post(
+      dsn.postUri,
+      headers: _credentialBuilder.configure(_headers),
+      body: body,
+    );
+
+    if (response.statusCode != 200) {
+      // body guard to not log the error as it has performance impact to allocate
+      // the body String.
+      if (_options.debug) {
+        _options.logger(
+          SentryLevel.error,
+          'API returned an error, statusCode = ${response.statusCode}, '
+          'body = ${response.body}',
+        );
+      }
+      return SentryId.empty();
+    } else {
+      _options.logger(
+        SentryLevel.debug,
+        'Event ${event.eventId} was sent suceffully.',
+      );
+    }
+
+    final eventId = json.decode(response.body)['id'];
+    return eventId != null ? SentryId.fromId(eventId) : SentryId.empty();
+  }
+
+  List<int> _bodyEncoder(
+    Map<String, dynamic> data,
+    Map<String, String> headers, {
+    bool compressPayload,
+  }) {
+    // [SentryIOClient] implement gzip compression
+    // gzip compression is not available on browser
+    var body = utf8.encode(json.encode(data));
+    if (compressPayload) {
+      body = compressBody(body, headers);
+    }
+    return body;
+  }
+}
+
+class CredentialBuilder {
+  final String _authHeader;
+
+  final ClockProvider clock;
+
+  int get timestamp => clock().millisecondsSinceEpoch;
+
+  CredentialBuilder({@required Dsn dsn, String clientId, @required this.clock})
+      : _authHeader = buildAuthHeader(
+          publicKey: dsn.publicKey,
+          secretKey: dsn.secretKey,
+          clientId: clientId,
+        );
+
+  static String buildAuthHeader({
+    String publicKey,
+    String secretKey,
+    String clientId,
+  }) {
+    var header = 'Sentry sentry_version=6, sentry_client=$clientId, '
+        'sentry_key=${publicKey}';
+
+    if (secretKey != null) {
+      header += ', sentry_secret=${secretKey}';
+    }
+
+    return header;
+  }
+
+  Map<String, dynamic> configure(Map<String, dynamic> headers) {
+    return headers
+      ..addAll(
+        <String, String>{
+          'X-Sentry-Auth': '$_authHeader, sentry_timestamp=${timestamp}'
+        },
+      );
+  }
+}
+
+Map<String, String> _buildHeaders({String sdkIdentifier}) {
+  final headers = {'Content-Type': 'application/json'};
+  // NOTE(lejard_h) overriding user agent on VM and Flutter not sure why
+  // for web it use browser user agent
+  if (!isWeb) {
+    headers['User-Agent'] = sdkIdentifier;
+  }
+  return headers;
+}

dart/lib/src/transport/noop_transport.dart

@@ -4,9 +4,6 @@ import '../protocol.dart';
 import 'transport.dart';
 
 class NoOpTransport implements Transport {
-  @override
-  Dsn get dsn => null;
-
   @override
   Future<SentryId> send(SentryEvent event) => Future.value(SentryId.empty());
 }

dart/lib/src/transport/transport.dart

@@ -1,122 +1,9 @@
 import 'dart:async';
-import 'dart:convert';
-
-import 'package:meta/meta.dart';
 
 import '../protocol.dart';
-import '../sentry_options.dart';
-import '../utils.dart';
-import 'noop_encode.dart' if (dart.library.io) 'encode.dart';
-
-/// A transport is in charge of sending the event to the Sentry server.
-class Transport {
-  final SentryOptions _options;
-
-  @visibleForTesting
-  final Dsn dsn;
-
-  /// Use for browser stacktrace
-  final String _origin;
-
-  CredentialBuilder _credentialBuilder;
-
-  final Map<String, String> _headers;
-
-  Transport({@required SentryOptions options, String origin})
-      : _options = options,
-        _origin = origin,
-        dsn = Dsn.parse(options.dsn),
-        _headers = _buildHeaders(sdkIdentifier: options.sdk.identifier) {
-    _credentialBuilder = CredentialBuilder(
-      dsn: Dsn.parse(options.dsn),
-      clientId: options.sdk.identifier,
-      clock: options.clock,
-    );
-  }
-
-  Future<SentryId> send(SentryEvent event) async {
-    final data = event.toJson(origin: _origin);
-
-    final body = _bodyEncoder(
-      data,
-      _headers,
-      compressPayload: _options.compressPayload,
-    );
-
-    final response = await _options.httpClient.post(
-      dsn.postUri,
-      headers: _credentialBuilder.configure(_headers),
-      body: body,
-    );
-
-    if (response.statusCode != 200) {
-      return SentryId.empty();
-    }
-
-    final eventId = json.decode(response.body)['id'];
-    return eventId != null ? SentryId.fromId(eventId) : SentryId.empty();
-  }
-
-  List<int> _bodyEncoder(
-    Map<String, dynamic> data,
-    Map<String, String> headers, {
-    bool compressPayload,
-  }) {
-    // [SentryIOClient] implement gzip compression
-    // gzip compression is not available on browser
-    var body = utf8.encode(json.encode(data));
-    if (compressPayload) {
-      body = compressBody(body, headers);
-    }
-    return body;
-  }
-}
-
-class CredentialBuilder {
-  final String _authHeader;
-
-  final ClockProvider clock;
-
-  int get timestamp => clock().millisecondsSinceEpoch;
-
-  CredentialBuilder({@required Dsn dsn, String clientId, @required this.clock})
-      : _authHeader = buildAuthHeader(
-          publicKey: dsn.publicKey,
-          secretKey: dsn.secretKey,
-          clientId: clientId,
-        );
-
-  static String buildAuthHeader({
-    String publicKey,
-    String secretKey,
-    String clientId,
-  }) {
-    var header = 'Sentry sentry_version=6, sentry_client=$clientId, '
-        'sentry_key=${publicKey}';
-
-    if (secretKey != null) {
-      header += ', sentry_secret=${secretKey}';
-    }
-
-    return header;
-  }
-
-  Map<String, dynamic> configure(Map<String, dynamic> headers) {
-    return headers
-      ..addAll(
-        <String, String>{
-          'X-Sentry-Auth': '$_authHeader, sentry_timestamp=${timestamp}'
-        },
-      );
-  }
-}
 
-Map<String, String> _buildHeaders({String sdkIdentifier}) {
-  final headers = {'Content-Type': 'application/json'};
-  // NOTE(lejard_h) overriding user agent on VM and Flutter not sure why
-  // for web it use browser user agent
-  if (!isWeb) {
-    headers['User-Agent'] = sdkIdentifier;
-  }
-  return headers;
+/// A transport is in charge of sending the event either via http
+/// or caching in the disk.
+abstract class Transport {
+  Future<SentryId> send(SentryEvent event);
 }