-
-
Notifications
You must be signed in to change notification settings - Fork 206
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
Android - implement handler strategy #3694
base: main
Are you sure you want to change the base?
Conversation
Assert.Equal(expected, options.HandlerStrategy); | ||
} | ||
[Fact] | ||
public void HandlerStrategy_set() |
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.
we're just testing setters here, can we add some integration test somehow?
Can we repro this null reference to SIGSEGV issue? We have tests in the repo where we have the SDK capture something and we grab the event from the transport and asset on what was created.
We'd add tests and #if ANDROID
. cc @jamescrosswell @vaind are DeviceTests alive? I know we had issues
If so, any points you could give @bricefriha on how to do this?
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.
cc @jamescrosswell @vaind are DeviceTests alive? I know we had issues
They're working fine on the main branch. We've got issues with them when we try to make them target net8.0-android or net8.0-ios... we don't have to do that until we bump to net9.0 though.
Co-authored-by: Bruno Garcia <[email protected]>
Co-authored-by: Bruno Garcia <[email protected]>
Co-authored-by: Bruno Garcia <[email protected]>
…oid.Init(AppContext, configuration);`
…coped namespace declaration
…Y_CHAIN_AT_START by default
…oesn't need to set the handler themself
From #1027, we had to add the handler strategy to the .NET SDK, from the sentry-java (7.15.0-alpha1).
This is also meant to fix #3461. I tested this, and it seems to be fixed now.
More context at:
Set handler strategy from the options
Note
These changes seem to require a pre-alpha release