-
-
Notifications
You must be signed in to change notification settings - Fork 257
Support user context #17
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from 7 commits
64dcd35
d94749c
99b20c0
750531a
f58b55e
1823205
5e022d4
2e01023
df7ebf8
c925605
3e568ca
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -143,6 +143,13 @@ class SentryClient { | |
/// Attached to the event payload. | ||
final String projectId; | ||
|
||
@visibleForTesting | ||
User _userContext; | ||
|
||
set userContext(User val) { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. We require dartdocs for all public API. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. done |
||
_userContext = val; | ||
} | ||
|
||
@visibleForTesting | ||
String get postUri => | ||
'${dsnUri.scheme}://${dsnUri.host}/api/$projectId/store/'; | ||
|
@@ -170,6 +177,10 @@ class SentryClient { | |
if (environmentAttributes != null) | ||
mergeAttributes(environmentAttributes.toJson(), into: data); | ||
|
||
// merge the user context | ||
if (_userContext != null) { | ||
mergeAttributes({'user': _userContext.toJson()}, into: data); | ||
} | ||
mergeAttributes(event.toJson(), into: data); | ||
|
||
List<int> body = utf8.encode(json.encode(data)); | ||
|
@@ -285,6 +296,7 @@ class Event { | |
this.tags, | ||
this.extra, | ||
this.fingerprint, | ||
this.userContext, | ||
}); | ||
|
||
/// The logger that logged the event. | ||
|
@@ -330,6 +342,8 @@ class Event { | |
/// they must be JSON-serializable. | ||
final Map<String, dynamic> extra; | ||
|
||
final User userContext; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. dartdocs There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. done |
||
|
||
/// Used to deduplicate events by grouping ones with the same fingerprint | ||
/// together. | ||
/// | ||
|
@@ -389,9 +403,45 @@ class Event { | |
|
||
if (extra != null && extra.isNotEmpty) json['extra'] = extra; | ||
|
||
Map<String, String> userContextMap; | ||
if (userContext != null && | ||
(userContextMap = userContext.toJson()).isNotEmpty) | ||
json['user'] = userContextMap; | ||
|
||
if (fingerprint != null && fingerprint.isNotEmpty) | ||
json['fingerprint'] = fingerprint; | ||
|
||
return json; | ||
} | ||
} | ||
|
||
/// Conforms to the User Interface contract for Sentry | ||
/// https://docs.sentry.io/clientdev/interfaces/user/ | ||
/// | ||
/// "user": { | ||
/// "id": "unique_id", | ||
/// "username": "my_user", | ||
/// "email": "[email protected]", | ||
/// "ip_address": "127.0.0.1", | ||
/// "subscription": "basic" | ||
///} | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. nit1: add one space after the There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. done |
||
/// | ||
class User { | ||
final String id; | ||
final String username; | ||
final String email; | ||
final String ipAddress; | ||
final String subscription; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. dartdocs for all public fields. Ideally, link to the docs on sentry.io. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. done |
||
|
||
const User(this.id, this.username, this.email, this.ipAddress, this.subscription); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. none of them are non-null. The docs say that you must have at least an ID OR an IP Address set. but that is up to the client app to enforce which they want. adding an assert to inform the developer during debug that there might be an issue. |
||
|
||
Map<String, String> toJson() { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. dartdocs There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. done |
||
return <String, String>{ | ||
"id": id, | ||
"username": username, | ||
"email": email, | ||
"ip_address": ipAddress, | ||
"subscription": subscription | ||
}; | ||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -171,10 +171,71 @@ void main() { | |
|
||
await client.close(); | ||
}); | ||
|
||
test('$Event userContext overrides client', () async { | ||
final MockClient httpMock = new MockClient(); | ||
final Clock fakeClock = new Clock.fixed(new DateTime(2017, 1, 2)); | ||
|
||
String loggedUserId; // used to find out what user context was sent | ||
httpMock.answerWith((Invocation invocation) async { | ||
if (invocation.memberName == #close) { | ||
return null; | ||
} | ||
if (invocation.memberName == #post) { | ||
// parse the body and detect which user context was sent | ||
var bodyData = invocation.namedArguments[new Symbol("body")]; | ||
var decoded = new Utf8Codec().decode(bodyData); | ||
var decodedJson = new JsonDecoder().convert(decoded); | ||
loggedUserId = decodedJson['user']['id']; | ||
return new Response('', 401, headers: <String, String>{ | ||
'x-sentry-error': 'Invalid api key', | ||
}); | ||
} | ||
fail('Unexpected invocation of ${invocation.memberName} in HttpMock'); | ||
}); | ||
|
||
const clientUserContext = const User( | ||
"client_user", "username", "[email protected]", "127.0.0.1", "basic"); | ||
final eventUserContext = new User( | ||
"event_user", "username", "[email protected]", "127.0.0.1", "basic"); | ||
|
||
final SentryClient client = new SentryClient( | ||
dsn: _testDsn, | ||
httpClient: httpMock, | ||
clock: fakeClock, | ||
uuidGenerator: () => 'X' * 32, | ||
compressPayload: false, | ||
environmentAttributes: const Event( | ||
serverName: 'test.server.com', | ||
release: '1.2.3', | ||
environment: 'staging', | ||
), | ||
); | ||
client.userContext = clientUserContext; | ||
|
||
try { | ||
throw new ArgumentError('Test error'); | ||
} catch (error, stackTrace) { | ||
final eventWithoutContext = | ||
new Event(exception: error, stackTrace: stackTrace); | ||
final eventWithContext = new Event( | ||
exception: error, | ||
stackTrace: stackTrace, | ||
userContext: eventUserContext); | ||
await client.capture(event: eventWithoutContext); | ||
expect(loggedUserId, clientUserContext.id); | ||
await client.capture(event: eventWithContext); | ||
expect(loggedUserId, eventUserContext.id); | ||
} | ||
|
||
await client.close(); | ||
}); | ||
}); | ||
|
||
group('$Event', () { | ||
test('serializes to JSON', () { | ||
final user = new User( | ||
"user_id", "username", "[email protected]", "127.0.0.1", "basic"); | ||
expect( | ||
new Event( | ||
message: 'test-message', | ||
|
@@ -190,6 +251,7 @@ void main() { | |
'g': 2, | ||
}, | ||
fingerprint: <String>[Event.defaultFingerprint, 'foo'], | ||
userContext: user, | ||
).toJson(), | ||
<String, dynamic>{ | ||
'platform': 'dart', | ||
|
@@ -203,6 +265,13 @@ void main() { | |
'tags': {'a': 'b', 'c': 'd'}, | ||
'extra': {'e': 'f', 'g': 2}, | ||
'fingerprint': ['{{ default }}', 'foo'], | ||
'user': { | ||
'id': 'user_id', | ||
'username': 'username', | ||
'email': '[email protected]', | ||
'ip_address': '127.0.0.1', | ||
'subscription': 'basic' | ||
}, | ||
}, | ||
); | ||
}); | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why opt for a mutable field rather than a final one, like all others?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
done