-
Notifications
You must be signed in to change notification settings - Fork 205
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
Support stopSession() and resumeSession() in NDK #432
Conversation
…ssion information in their payl
Allow sessions to be stopped on the NDK. when a session is stopped from the java layer, the session information will no longer be serialised in the error payload of fatal NDK errors.
the START_SESSION message is emitted when a session is started or resumed, and allows the NDK layer to update its cached value for session information which is included in the error payload. It was previously assumed that the handled count would always be 0 when starting a session - this is no longer the case as sessions can be resumed, therefore the handled count is passed as part of the message.
e07c7fa
to
3fd8417
Compare
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.
Looks good, added two small improvements.
} | ||
|
||
void stopSession() { | ||
Session session = currentSession.get(); | ||
|
||
if (session != null) { | ||
session.isStopped.set(true); | ||
setChanged(); | ||
notifyObservers(new NativeInterface.Message( |
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.
There should be an ObserverInterfaceTest
covering STOP_SESSION
events.
@@ -326,6 +334,10 @@ private void handleStartSession(Object arg) { | |||
warn("START_SESSION object is invalid: " + arg); | |||
} | |||
|
|||
private void handleStopSession() { |
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.
This method, while consistent with many of the others, is redundant. Could instead just call stoppedSession()
above, similar to clearBreadcrumbs()
and addHandledEvent()
, which also don't need additional processing before crossing to the native layer.
Goal
Adds the ability to stop and resume sessions in the NDK layer. When a session is stopped, session information will not be added to error payloads and handled/unhandled error counts will not be incremented.
For fatal NDK errors, we cannot call the JNI without crashing the process, so we write an error report to disk using C. This PR updates the serialisation code to handle the concept of a stopped session.
See #429 for the Java implementation, and further details.
Changeset
SESSION_STARTED
observable event fromSessionTracker.java
when a session is started or resumed, which ultimately sets session information on the NDK payload.SESSION_STOPPED
observable event, which sets a boolean flag onbugsnag_report
that prevents serialisation of session information in fatal NDK error reportsSESSION_STARTED
event to also include the handled count, rather than assuming it will always equal 0.Tests
Added mazerunner scenarios to verify that session information is not present in error payloads of a stopped session.