Skip to content

Support setting error type on individual stack frames #1001

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

Conversation

fractalwrench
Copy link
Contributor

@fractalwrench fractalwrench commented Nov 17, 2020

Goal

Supports setting the error type on individual stackframes by setting the Bugsnag-Stacktrace-Types header. This allows the pipeline to distinguish events and process them accordingly. Supported values are android, c, reactnativejs. These are set as a comma delimited string.

Changeset

  • Updated errorApiHeaders() to set Bugsnag-Stacktrace-Types header if the value has been set
  • Calculated value of Bugsnag-Stacktrace-Types from the event (by checking all the types set on every stackframe) and from the file (by checking encoded info in the filename)
  • Updated EventStore to encode the error types in the event file
  • The AnrPlugin now updates the thread.0.stacktrace when capturing an error - previously only errors.0.stacktrace was updated, which lead to a disparity in information on the dashboard
  • Created EventFilenameInfo file which holds classes/functions responsible for encoding/decoding event information into an Event

Testing

  • Added unit tests to verify error API headers, the error type calculation, and the event store filename
  • Updated existing stackframe serialization tests to verify type is serialized
  • Enhanced existing E2E scenarios to include checks on type field
  • Verified that ANRs set their error type accordingly via E2E scenarios

@fractalwrench fractalwrench changed the title Plat 5420/stacktraces Support setting error type on individual stack frames Nov 17, 2020
@bugsnagbot
Copy link
Collaborator

bugsnagbot commented Nov 17, 2020

Android notifier sizes

Format Size impact of Bugsnag (kB) Size impact of Bugsnag when Minified (kB)
APK 1447.49 1364.77
arm64_v8a 373.35 291.43
armeabi 352.86 266.85
armeabi_v7a 336.49 250.46
x86 414.29 328.28
x86_64 393.82 311.9

Generated by 🚫 Danger

@fractalwrench fractalwrench marked this pull request as ready for review November 17, 2020 15:15
@fractalwrench fractalwrench force-pushed the PLAT-5420/stacktraces branch 2 times, most recently from ab3a2a2 to 9ba924a Compare November 18, 2020 10:09
Copy link
Contributor

@tomlongridge tomlongridge left a comment

Choose a reason for hiding this comment

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

The API spec, and the intent of the change, is for the field says that the frame type should be set where it doesn't match the error type. I don't think we should be setting the type for all NDK frames - only when it's been set explicitly either in a callback or by one of the new features.

@fractalwrench fractalwrench changed the base branch from integration/capture-ndk-stack-traces-for-anrs to 5393-native-anr-scenarios November 20, 2020 15:05
Base automatically changed from 5393-native-anr-scenarios to integration/capture-ndk-stack-traces-for-anrs November 20, 2020 16:02
@fractalwrench
Copy link
Contributor Author

Updated changeset so that the type is only set on individual stackframes when the error object is mixed (as with native ANRs). E2E scenarios have also been added to verify this.

This should be ready for another round of review, pending the results of CI.

@fractalwrench fractalwrench force-pushed the PLAT-5420/stacktraces branch 2 times, most recently from 977187c to 47cc1b0 Compare November 23, 2020 09:26
Copy link
Contributor

@tomlongridge tomlongridge left a comment

Choose a reason for hiding this comment

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

Looks much cleaner 👍 I've made some suggestions from the point-of-view of someone reading this for the first time, to make the abstraction cleaner.

Verified

This commit was created on GitHub.com and signed with GitHub’s verified signature. The key has expired.
Assert that exception stacktrace is equal to thread stacktrace
@fractalwrench fractalwrench merged commit 5461777 into integration/capture-ndk-stack-traces-for-anrs Nov 24, 2020
@fractalwrench fractalwrench deleted the PLAT-5420/stacktraces branch November 24, 2020 13:47
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants