Skip to content
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

proposal: x/mobile: better support for unrecovered panics #70668

Open
xcolwell opened this issue Dec 3, 2024 · 2 comments
Open

proposal: x/mobile: better support for unrecovered panics #70668

xcolwell opened this issue Dec 3, 2024 · 2 comments
Labels
Milestone

Comments

@xcolwell
Copy link

xcolwell commented Dec 3, 2024

Proposal Details

Currently with gomobile, the following example would crash the entire app with a single error message and no stack trace.

func HelloWorld() {
  var x *SomeType
  x.Foo()
  // oh no, the application crashed
}

In mobile applications we typically don't want to crash the app, but instead capture and report unrecovered errors and then handle the unrecovered errors as part of error handling in the caller. I'd like to propose four improvement to gomobile:

  1. Standard stack trace logging for all unrecovered errors.
  2. A global unrecovered error callback that is called before propagating unrecovered errors.
  3. Instead of unrecovered errors crashing the application, the error should be converted to an unchecked exception/fatal error type in the caller code so that it can integrate with the native unhandled error flow.
  4. Support for propagating unrecovered errors to the caller as checked exceptions. In many cases a typical return will be a pair of value and error, which is correctly translated to the exception framework by gomobile. Unrecovered errors would more naturally be "thrown" to the caller with the correct stack trace than crashing the application.

Is there a process to vet improvements like this before working on a PR?

@ignoramous
Copy link

Unsure about passing unrecovered errors from Go into Java as Exceptions (instead of Errors). Not all errors if recovered, leave the Go client code in a desirable state. For ex:

# untested if the below compiles

type obj struct {
  sync.RWMutex
  boom *int
}

func (a *obj) neverUnlocksIfPanicsRecovered() {
   a.Lock()
   (*a.boom)++
   a.Unlock()
}

func entrypoint() {
  a := new(obj)
  GoMobileExec(a.neverUnlocksIfPanicsRecovered) // deadlocks
  GoMobileExec(a.neverUnlocksIfPanicsRecovered)
}

func GoMobileExec(f func()) {
  defer func() {
    recovered := recover()
    if recovered != nil { /*error up to JNI?*/ }
  }()
  
  f()
}

Might also have to set debug.SetPanicOnFault to turn faults into recoverable panics.

@xcolwell
Copy link
Author

xcolwell commented Dec 8, 2024

Thanks for the feedback @ignoramous. I see your point about bad state, but generally isn't cleaning up state on panics a concern for any robust code? To idiom we use is to always clean up state with a defer.

I'm curious how you (or anyone who has been deploying apps with Gomobile) are working with unrecovered panics/errors. I've heard some feedback that doing it in a custom way per app is the way to go, because of different styles like what you mentioned. The default logging is unusable, but perhaps every app needs to just implement their own approach?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
Status: Incoming
Development

No branches or pull requests

3 participants