Skip to content
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
40 commits
Select commit Hold shift + click to select a range
a10d740
remove the SentryResponse class
rxlabz Oct 21, 2020
623c904
refactor : add a Transport class
rxlabz Oct 21, 2020
0dcb613
refactor : move the clock initialization to Transport
rxlabz Oct 21, 2020
c97c98b
- refactor : applyScope
rxlabz Oct 21, 2020
477fdac
changelog
rxlabz Oct 21, 2020
b8fb322
fix merge conflicts
rxlabz Oct 22, 2020
fd92e3f
Merge branch 'feature/unified-api' into ref/transport
rxlabz Oct 22, 2020
0944c68
move the headers definition strategy to transport
rxlabz Oct 22, 2020
46829fc
- Instantiate Transport from options
rxlabz Oct 22, 2020
01d4e19
handle event dropping in event processors
rxlabz Oct 22, 2020
fbd001e
fix client._applyScope : merge event and scope tags and extra
rxlabz Oct 22, 2020
4d39531
remove client.toString()
rxlabz Oct 22, 2020
34fb528
fix a test
rxlabz Oct 22, 2020
2437fa4
Merge branch 'feature/unified-api' into ref/transport
rxlabz Oct 22, 2020
57798f5
handle the event serialization in the transport layer
rxlabz Oct 22, 2020
789e154
Merge remote-tracking branch 'origin/feature/unified-api' into ref/tr…
rxlabz Oct 22, 2020
1aa05d6
remove stackFrameFilter references
rxlabz Oct 22, 2020
9895142
Merge remote-tracking branch 'origin/feature/unified-api' into ref/tr…
rxlabz Oct 22, 2020
cb5e309
add a CredentialBuilder
rxlabz Oct 22, 2020
e563d2c
- headers configuration
rxlabz Oct 23, 2020
71b53c8
- fix scope.level precedence
rxlabz Oct 23, 2020
be3f0a2
buildHeaders and fix timestamp test
rxlabz Oct 23, 2020
43b798c
- add a isWeb helper to define the event.platform
rxlabz Oct 23, 2020
19dc91d
feat: apply sample rate
marandaneto Oct 23, 2020
cec1f21
changelog
marandaneto Oct 23, 2020
909d4f9
- add a isWeb helper to define the event.platform and sdkName
rxlabz Oct 23, 2020
8b600dc
- add a isWeb helper to define the event.platform and sdkName
rxlabz Oct 23, 2020
4ff58a6
Merge remote-tracking branch 'origin/feat/sample-rate' into ref/trans…
rxlabz Oct 23, 2020
5493e19
fix
marandaneto Oct 23, 2020
621d6c4
update tests
rxlabz Oct 23, 2020
e11f653
Merge remote-tracking branch 'origin/feat/sample-rate' into ref/trans…
rxlabz Oct 23, 2020
88526e2
remove the platform arg from Client ctor
rxlabz Oct 23, 2020
796b633
set client.options private
rxlabz Oct 23, 2020
fb8b9e0
remove event.platform redefinition
rxlabz Oct 23, 2020
1dbcba4
set options.sdk if null
rxlabz Oct 23, 2020
8b56bce
remove fields from Transport Api
rxlabz Oct 23, 2020
daeb29e
private version consts
rxlabz Oct 23, 2020
098d58c
remove transport from the Client ctor
rxlabz Oct 23, 2020
2833a94
remove options.environmentAttributes
rxlabz Oct 23, 2020
35671d4
event processing : platform
rxlabz Oct 23, 2020
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Prev Previous commit
Next Next commit
- Instantiate Transport from options
- move the postUri logic to Dsn
  • Loading branch information
rxlabz committed Oct 22, 2020
commit 46829fc4f6f2f03d7da8708f576826e2501b091c
13 changes: 6 additions & 7 deletions dart/lib/src/browser_client.dart
Original file line number Diff line number Diff line change
Expand Up @@ -42,15 +42,14 @@ class SentryBrowserClient extends SentryClient {
);
}

SentryBrowserClient._(SentryOptions options,
{String origin, @required String platform})
: super.base(
SentryBrowserClient._(
SentryOptions options, {
String origin,
@required String platform,
}) : super.base(
options,
transport: Transport(
dsn: options.dsn,
httpClient: options.httpClient,
clock: options.clock,
compressPayload: false,
options: options,
sdk: Sdk(name: browserSdkName, version: sdkVersion),
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

options.sdkVersion already exists, but its probably null, we'd need to find a way to instantiate it based on the platform (dart, flutter or web), but ideally, we just read this field from the options here

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

on a second thought, shouldn't this be just either: sentry.dart or sentry.flutter? Given that this is what the user installed at the end?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

if we need only the identifier of the client, yes, only a String would be fine.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

✅ fixed

platform: platform,
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

wondering if this is the same as options.sentryClientName, could you check it?

Copy link
Contributor Author

@rxlabz rxlabz Oct 23, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

platform is 'dart' or 'javascript' , according to the docs options.sentryClientName is 'sentry.{language}.{platform}/{version}'

Copy link
Contributor

@marandaneto marandaneto Oct 23, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

we could do this then?

factory SentryIOClient(SentryOptions options) {
    options.platform = sdkPlatform;
    SentryIOClient._(options);
}

same for the WebClient, so we move this flag to the options and the transport don't need to take it as a param? we do it automatically.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

✅ platform is now defined at runtime

origin: origin,
Expand Down
9 changes: 1 addition & 8 deletions dart/lib/src/io_client.dart
Original file line number Diff line number Diff line change
Expand Up @@ -25,13 +25,6 @@ class SentryIOClient extends SentryClient {
SentryIOClient._(SentryOptions options, {@required String platform})
: super.base(
options,
transport: Transport(
compressPayload: options.compressPayload,
httpClient: options.httpClient,
clock: options.clock,
sdk: sdk,
dsn: options.dsn,
platform: platform,
),
transport: Transport(options: options, sdk: sdk, platform: platform),
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

we set the transport to the options and then pass the options along, so transport wont be null

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

just to remember: we might be able to identify if its web or not on runtime via https://api.flutter.dev/flutter/foundation/kIsWeb-constant.html
so transport wont even need a platform param here, it could be already on sentry options ctor for example

);
}
31 changes: 31 additions & 0 deletions dart/lib/src/protocol/dsn.dart
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,26 @@ class Dsn {
/// The DSN URI.
final Uri uri;

String get postUri {
final port = uri.hasPort &&
((uri.scheme == 'http' && uri.port != 80) ||
(uri.scheme == 'https' && uri.port != 443))
? ':${uri.port}'
: '';

final pathLength = uri.pathSegments.length;

String apiPath;
if (pathLength > 1) {
// some paths would present before the projectID in the uri
apiPath =
(uri.pathSegments.sublist(0, pathLength - 1) + ['api']).join('/');
} else {
apiPath = 'api';
}
return '${uri.scheme}://${uri.host}$port/$apiPath/$projectId/store/';
}

static Dsn parse(String dsn) {
final uri = Uri.parse(dsn);
final userInfo = uri.userInfo.split(':');
Expand All @@ -45,4 +65,15 @@ class Dsn {
uri: uri,
);
}

String buildAuthHeader({int timestamp, String clientId}) {
var header = 'Sentry sentry_version=6, sentry_client=$clientId, '
'sentry_timestamp=$timestamp, sentry_key=${publicKey}';

if (secretKey != null) {
header += ', sentry_secret=${secretKey}';
}

return header;
}
}
88 changes: 20 additions & 68 deletions dart/lib/src/transport/transport.dart
Original file line number Diff line number Diff line change
@@ -1,7 +1,6 @@
import 'dart:async';
import 'dart:convert';

import 'package:http/http.dart';
import 'package:meta/meta.dart';
import 'package:sentry/src/utils.dart';

Expand All @@ -18,92 +17,45 @@ typedef BodyEncoder = List<int> Function(

/// A transport is in charge of sending the event to the Sentry server.
class Transport {
final Client httpClient;
final SentryOptions _options;

final Dsn _dsn;

final Sdk sdk;
@visibleForTesting
final Dsn dsn;

final bool compressPayload;
/// Use for browser stacktrace
final String origin;

/// Used by sentry to differentiate browser from io environment
final String platform;

final ClockProvider _clock;

/// Use for browser stacktrace
final String origin;
final Sdk sdk;

Transport({
@required String dsn,
@required this.compressPayload,
@required this.httpClient,
@required SentryOptions options,
@required this.sdk,
@required ClockProvider clock,
@required this.platform,
this.origin,
}) : _dsn = Dsn.parse(dsn),
_clock = clock ?? getUtcDateTime;

/// The DSN URI.
@visibleForTesting
Uri get dsnUri => _dsn.uri;

/// The Sentry.io public key for the project.
@visibleForTesting
// ignore: invalid_use_of_visible_for_testing_member
String get publicKey => _dsn.publicKey;

/// The Sentry.io secret key for the project.
@visibleForTesting
// ignore: invalid_use_of_visible_for_testing_member
String get secretKey => _dsn.secretKey;

/// The ID issued by Sentry.io to your project.
///
/// Attached to the event payload.
String get projectId => _dsn.projectId;

String get clientId => sdk.identifier;

@visibleForTesting
String get postUri {
final port = dsnUri.hasPort &&
((dsnUri.scheme == 'http' && dsnUri.port != 80) ||
(dsnUri.scheme == 'https' && dsnUri.port != 443))
? ':${dsnUri.port}'
: '';

final pathLength = dsnUri.pathSegments.length;

String apiPath;
if (pathLength > 1) {
// some paths would present before the projectID in the dsnUri
apiPath =
(dsnUri.pathSegments.sublist(0, pathLength - 1) + ['api']).join('/');
} else {
apiPath = 'api';
}
return '${dsnUri.scheme}://${dsnUri.host}$port/$apiPath/$projectId/store/';
}
}) : _options = options,
dsn = Dsn.parse(options.dsn);

Future<SentryId> send(Map<String, dynamic> data) async {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

our Transport still takes a typed object, SentryEvent in the case, and only here in the transport we'd convert to a json-like object
https://github.com/getsentry/sentry-java/blob/main/sentry/src/main/java/io/sentry/transport/ITransport.java#L11

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

✅ fixed

final now = _clock();
final now = _options.clock();

var authHeader = 'Sentry sentry_version=6, sentry_client=$clientId, '
'sentry_timestamp=${now.millisecondsSinceEpoch}, sentry_key=$publicKey';
if (secretKey != null) {
authHeader += ', sentry_secret=$secretKey';
}
var authHeader = dsn.buildAuthHeader(
timestamp: now.millisecondsSinceEpoch, clientId: sdk.identifier);

mergeAttributes(_getContext(now), into: data);

final headers = buildHeaders(authHeader, sdk: sdk);

final body = bodyEncoder(data, headers, compressPayload: compressPayload);
final body = bodyEncoder(
data,
headers,
compressPayload: _options.compressPayload,
);

final response = await httpClient.post(
postUri,
final response = await _options.httpClient.post(
dsn.postUri,
headers: headers,
body: body,
);
Expand All @@ -117,7 +69,7 @@ class Transport {
}

Map<String, dynamic> _getContext(DateTime now) => {
'project': projectId,
'project': dsn.projectId,
'timestamp': formatDateAsIso8601WithSecondPrecision(now),
'platform': platform,
};
Expand Down
44 changes: 23 additions & 21 deletions dart/test/test_utils.dart
Original file line number Diff line number Diff line change
Expand Up @@ -91,7 +91,7 @@ Future testCaptureException(
expect('$sentryId', 'testeventid');
}

expect(postUri, client.transport.postUri);
expect(postUri, client.transport.dsn.postUri);

testHeaders(
headers,
Expand Down Expand Up @@ -182,46 +182,48 @@ Future testCaptureException(
void runTest({Codec<List<int>, List<int>> gzip, bool isWeb = false}) {
test('can parse DSN', () async {
final client = SentryClient(SentryOptions(dsn: testDsn));
expect(client.transport.dsnUri, Uri.parse(testDsn));
expect(client.transport.postUri, 'https://sentry.example.com/api/1/store/');
expect(client.transport.publicKey, 'public');
expect(client.transport.secretKey, 'secret');
expect(client.transport.projectId, '1');
expect(client.transport.dsn.uri, Uri.parse(testDsn));
expect(client.transport.dsn.postUri,
'https://sentry.example.com/api/1/store/');
expect(client.transport.dsn.publicKey, 'public');
expect(client.transport.dsn.secretKey, 'secret');
expect(client.transport.dsn.projectId, '1');
await client.close();
});

test('can parse DSN without secret', () async {
final client = SentryClient(SentryOptions(dsn: _testDsnWithoutSecret));
expect(client.transport.dsnUri, Uri.parse(_testDsnWithoutSecret));
expect(client.transport.postUri, 'https://sentry.example.com/api/1/store/');
expect(client.transport.publicKey, 'public');
expect(client.transport.secretKey, null);
expect(client.transport.projectId, '1');
expect(client.transport.dsn.uri, Uri.parse(_testDsnWithoutSecret));
expect(client.transport.dsn.postUri,
'https://sentry.example.com/api/1/store/');
expect(client.transport.dsn.publicKey, 'public');
expect(client.transport.dsn.secretKey, null);
expect(client.transport.dsn.projectId, '1');
await client.close();
});

test('can parse DSN with path', () async {
final client = SentryClient(SentryOptions(dsn: _testDsnWithPath));
expect(client.transport.dsnUri, Uri.parse(_testDsnWithPath));
expect(client.transport.dsn.uri, Uri.parse(_testDsnWithPath));
expect(
client.transport.postUri,
client.transport.dsn.postUri,
'https://sentry.example.com/path/api/1/store/',
);
expect(client.transport.publicKey, 'public');
expect(client.transport.secretKey, 'secret');
expect(client.transport.projectId, '1');
expect(client.transport.dsn.publicKey, 'public');
expect(client.transport.dsn.secretKey, 'secret');
expect(client.transport.dsn.projectId, '1');
await client.close();
});
test('can parse DSN with port', () async {
final client = SentryClient(SentryOptions(dsn: _testDsnWithPort));
expect(client.transport.dsnUri, Uri.parse(_testDsnWithPort));
expect(client.transport.dsn.uri, Uri.parse(_testDsnWithPort));
expect(
client.transport.postUri,
client.transport.dsn.postUri,
'https://sentry.example.com:8888/api/1/store/',
);
expect(client.transport.publicKey, 'public');
expect(client.transport.secretKey, 'secret');
expect(client.transport.projectId, '1');
expect(client.transport.dsn.publicKey, 'public');
expect(client.transport.dsn.secretKey, 'secret');
expect(client.transport.dsn.projectId, '1');
await client.close();
});
test('sends client auth header without secret', () async {
Expand Down