-
Notifications
You must be signed in to change notification settings - Fork 130
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
On by default session tracking #286
Changes from 17 commits
831f63d
dceac81
b6d16bf
48e267f
b3d5420
abb08c2
4b7277d
f3922c6
26a2c37
57722ed
0645bad
8980ea3
0677b66
fda541e
e12ff70
d813cef
c89979d
ec307f1
e84d311
0ff92ec
57044de
348e095
3bf9f72
8a65049
f4bffac
19d23a4
70c6e76
00bb48b
8bd994f
d05198d
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 |
---|---|---|
|
@@ -73,10 +73,6 @@ typedef NSDictionary *_Nullable (^BugsnagBeforeNotifyHook)( | |
* The API key of a Bugsnag project | ||
*/ | ||
@property(readwrite, retain, nullable) NSString *apiKey; | ||
/** | ||
* The URL used to notify Bugsnag | ||
*/ | ||
@property(readwrite, retain, nullable) NSURL *notifyURL; | ||
/** | ||
* The release stage of the application, such as production, development, beta | ||
* et cetera | ||
|
@@ -142,16 +138,46 @@ BugsnagBreadcrumbs *breadcrumbs; | |
@property BOOL autoNotify; | ||
|
||
/** | ||
* Determines whether app sessions should be tracked automatically. By default this value is false. | ||
* Determines whether app sessions should be tracked automatically. By default this value is true. | ||
*/ | ||
@property BOOL shouldAutoCaptureSessions; | ||
|
||
/** | ||
* Set the endpoint to which tracked sessions reports are sent. This defaults to https://sessions.bugsnag.com, | ||
* but should be overridden if you are using Bugsnag On-premise, to point to your own Bugsnag endpoint. | ||
* Retrieves the endpoint used to notify Bugsnag of errors | ||
* | ||
* NOTE: it is strongly recommended that you set this value via setEndpointsForNotify:sessions: instead. | ||
* | ||
* @see setEndpointsForNotify:sessions: | ||
*/ | ||
@property(readwrite, retain, nullable) NSURL *notifyURL; | ||
|
||
/** | ||
* Retrieves the endpoint used to send tracked sessions to Bugsnag | ||
* | ||
* NOTE: it is strongly recommended that you set this value via setEndpointsForNotify:sessions: instead. | ||
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. is it worth using something like this to make this as deprecated? |
||
* | ||
* @see setEndpointsForNotify:sessions: | ||
*/ | ||
@property(readwrite, retain, nullable) NSURL *sessionURL; | ||
|
||
/** | ||
* Set the endpoints to send data to. By default we'll send error reports to | ||
* https://notify.bugsnag.com, and sessions to https://sessions.bugsnag.com, but you can | ||
* override this if you are using Bugsnag Enterprise to point to your own Bugsnag endpoint. | ||
* | ||
* Please note that it is recommended that you set both endpoints. If the notify endpoint is | ||
* missing, an assertion will be thrown. If the session endpoint is missing, a warning will be | ||
* logged and sessions will not be sent automatically. | ||
* | ||
* @param notify the notify endpoint | ||
* @param sessions the sessions endpoint | ||
* | ||
* @throws an assertion if the notify endpoint is not a valid URL | ||
*/ | ||
|
||
- (void)setEndpointsForNotify:(NSString *_Nonnull)notify | ||
sessions:(NSString *_Nonnull)sessions NS_SWIFT_NAME(setEndpoints(notify:sessions:)); | ||
|
||
/** | ||
* Set user metadata | ||
* | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -14,7 +14,7 @@ @implementation BugsnagUser | |
- (instancetype)initWithDictionary:(NSDictionary *)dict { | ||
if (self = [super init]) { | ||
_userId = dict[@"id"]; | ||
_emailAddress = dict[@"emailAddress"]; | ||
_emailAddress = dict[@"email"]; | ||
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. are the changes in this file related to this PR? 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. This addresses an issue in the session request serialisation which was found when adding mazerunner scenarios. Currently the session request sends I can split this off into a separate PR if you think it makes more sense. 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. that makes sense, I think it would be good to have it in a separate PR to track it more easily |
||
_name = dict[@"name"]; | ||
} | ||
return self; | ||
|
@@ -34,12 +34,12 @@ + (instancetype)userWithUserId:(NSString *)userId name:(NSString *)name emailAdd | |
return [[self alloc] initWithUserId:userId name:name emailAddress:emailAddress]; | ||
} | ||
|
||
|
||
- (NSDictionary *)toJson { | ||
NSMutableDictionary *dict = [NSMutableDictionary new]; | ||
BSGDictInsertIfNotNil(dict, self.userId, @"id"); | ||
BSGDictInsertIfNotNil(dict, self.emailAddress, @"emailAddress"); | ||
BSGDictInsertIfNotNil(dict, self.emailAddress, @"email"); | ||
BSGDictInsertIfNotNil(dict, self.name, @"name"); | ||
return [NSDictionary dictionaryWithDictionary:dict]; | ||
} | ||
|
||
@end |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,5 +1,6 @@ | ||
#import "Bugsnag.h" | ||
#import "BugsnagConfiguration.h" | ||
#import "BugsnagSessionTracker.h" | ||
#import "BugsnagUser.h" | ||
|
||
#import <XCTest/XCTest.h> | ||
|
@@ -56,7 +57,7 @@ - (void)testNotifyReleaseStagesExcludedSkipsSending { | |
|
||
- (void)testDefaultSessionConfig { | ||
BugsnagConfiguration *config = [BugsnagConfiguration new]; | ||
XCTAssertFalse([config shouldAutoCaptureSessions]); | ||
XCTAssertTrue([config shouldAutoCaptureSessions]); | ||
} | ||
|
||
- (void)testErrorApiHeaders { | ||
|
@@ -87,6 +88,61 @@ - (void)testSessionEndpoints { | |
XCTAssertEqualObjects(endpoint, config.sessionURL); | ||
} | ||
|
||
- (void)testNotifyEndpoint { | ||
BugsnagConfiguration *config = [BugsnagConfiguration new]; | ||
XCTAssertEqualObjects([NSURL URLWithString:@"https://notify.bugsnag.com/"], config.notifyURL); | ||
NSURL *endpoint = [NSURL URLWithString:@"http://localhost:8000"]; | ||
config.notifyURL = endpoint; | ||
XCTAssertEqualObjects(endpoint, config.notifyURL); | ||
} | ||
|
||
- (void)testSetEndpoints { | ||
BugsnagConfiguration *config = [BugsnagConfiguration new]; | ||
[config setEndpointsForNotify:@"http://notify.example.com" sessions:@"http://sessions.example.com"]; | ||
XCTAssertEqualObjects([NSURL URLWithString:@"http://notify.example.com"], config.notifyURL); | ||
XCTAssertEqualObjects([NSURL URLWithString:@"http://sessions.example.com"], config.sessionURL); | ||
} | ||
|
||
- (void)testSetEmptyNotifyEndpoint { | ||
@try { | ||
BugsnagConfiguration *config = [BugsnagConfiguration new]; | ||
[config setEndpointsForNotify:@"" sessions:@"http://sessions.example.com"]; | ||
XCTFail(); | ||
} @catch(NSException * e) { | ||
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. is it better to use something like |
||
} | ||
} | ||
|
||
- (void)testSetMalformedNotifyEndpoint { | ||
@try { | ||
BugsnagConfiguration *config = [BugsnagConfiguration new]; | ||
[config setEndpointsForNotify:@"http://" sessions:@"http://sessions.example.com"]; | ||
XCTFail(); | ||
} @catch(NSException * e) { | ||
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. is it better to use something like |
||
} | ||
} | ||
|
||
- (void)testSetEmptySessionsEndpoint { | ||
BugsnagConfiguration *config = [BugsnagConfiguration new]; | ||
[config setEndpointsForNotify:@"http://notify.example.com" sessions:@""]; | ||
BugsnagSessionTracker *sessionTracker | ||
= [[BugsnagSessionTracker alloc] initWithConfig:config apiClient:nil callback:nil]; | ||
|
||
XCTAssertNil(sessionTracker.currentSession); | ||
[sessionTracker startNewSession:[NSDate date] withUser:nil autoCaptured:NO]; | ||
XCTAssertNil(sessionTracker.currentSession); | ||
} | ||
|
||
- (void)testSetMalformedSessionsEndpoint { | ||
BugsnagConfiguration *config = [BugsnagConfiguration new]; | ||
[config setEndpointsForNotify:@"http://notify.example.com" sessions:@"f"]; | ||
BugsnagSessionTracker *sessionTracker | ||
= [[BugsnagSessionTracker alloc] initWithConfig:config apiClient:nil callback:nil]; | ||
|
||
XCTAssertNil(sessionTracker.currentSession); | ||
[sessionTracker startNewSession:[NSDate date] withUser:nil autoCaptured:NO]; | ||
XCTAssertNil(sessionTracker.currentSession); | ||
} | ||
|
||
- (void)testUser { | ||
BugsnagConfiguration *config = [BugsnagConfiguration new]; | ||
XCTAssertNil(config.currentUser); | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -19,7 +19,7 @@ - (void)testDictDeserialisation { | |
|
||
NSDictionary *dict = @{ | ||
@"id": @"test", | ||
@"emailAddress": @"[email protected]", | ||
@"email": @"[email protected]", | ||
This comment was marked as outdated.
Sorry, something went wrong. |
||
@"name": @"Tom Bombadil" | ||
}; | ||
BugsnagUser *user = [[BugsnagUser alloc] initWithDictionary:dict]; | ||
|
@@ -41,7 +41,7 @@ - (void)testPayloadSerialisation { | |
XCTAssertEqual(3, [rootNode count]); | ||
|
||
XCTAssertEqualObjects(@"test", rootNode[@"id"]); | ||
XCTAssertEqualObjects(@"[email protected]", rootNode[@"emailAddress"]); | ||
XCTAssertEqualObjects(@"[email protected]", rootNode[@"email"]); | ||
XCTAssertEqualObjects(@"Tom Bombadil", rootNode[@"name"]); | ||
} | ||
|
||
|
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.
is it worth using something like this to make this as deprecated?
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.
That's one of the outstanding questions - what are your thoughts on this? If we add
setEndpointsForNotify:sessions:
, then developers are able to set the value, but won't be able to read it withoutnotifyURL
andsessionsURL
.The way I see it there are a couple of options:
notifyURL
andsessionsURL
as is and document they shouldn't be setI've currently leant towards the first option.
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.
I see the dilemma. Is it possible to split the property up and only deprecate the write? I only jumped on this as I saw in the android notifier that we were deprecating bits like this as part of similar changes. If it doesn't make sense for
bugsnag-cocoa
then that is coolThere 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.
Might be worth trying:
readonly