-
Notifications
You must be signed in to change notification settings - Fork 104
Exception reporter disentangling for Android UI code #1345
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
@@ -3,6 +3,33 @@ package com.squareup.workflow1.internal | |||
import kotlin.contracts.ExperimentalContracts | |||
import kotlin.contracts.contract | |||
|
|||
/** | |||
* Like Kotlin's [requireNotNull], but uses [stackTraceKey] to create a fake top element | |||
* on the stack trace, ensuring that BugSnag's default grouping will create unique |
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.
nit: You could generalize this to "any crash reporter that uses stacktrace" rather than particular Bugsnag.
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.
Thanks, accidental copy paste from the internal method.
* @throws IllegalArgumentException if the [value] is false. | ||
*/ | ||
@OptIn(ExperimentalContracts::class) | ||
inline fun <T : Any> requireNotNullWithKey( |
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 think you'll have to mark this public
for explicit API mode, right?
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.
Yup. I wonder why the build didn't catch that?
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.
Oooh, I wonder if we turned off that check for internal
package. If so I think I can undo the other explicit publics I added in here. Trying it out…
6ab9934
to
2e1add9
Compare
Uses `Throwable.withKey` in a few more choke points, to break up crash reporter kitchen sinks. Had to make a few `internal` core methods public to allow their use from the android modules, but I think that's okay -- they're still in a package named `internal`, our intent should be clear.
2e1add9
to
0495048
Compare
Uses
Throwable.withKey
in a few more choke points, to break up crash reporter kitchen sinks. Had to make a fewinternal
core methods public to allow their use from the android modules, but I think that's okay -- they're still in a package namedinternal
, our intent should be clear.