Skip to content
This repository was archived by the owner on Feb 26, 2023. It is now read-only.

Fixed #646 sending background exception to the global UncaughtExceptionHandler#763

Merged
DayS merged 1 commit intoandroidannotations:developfrom
JoanZapata:develop
Oct 18, 2013
Merged

Fixed #646 sending background exception to the global UncaughtExceptionHandler#763
DayS merged 1 commit intoandroidannotations:developfrom
JoanZapata:develop

Conversation

@JoanZapata
Copy link
Copy Markdown
Contributor

See #646 and #727

@tbruyelle
Copy link
Copy Markdown
Contributor

Tested in my project where I set a custom exception handler (like ACRA and ACRA-like libraries do) and it works like a charm. The runtime exceptions thrown in @Background annotated methods are handled by the exception handler.

I also tested without custom exception handler, which then is the android one com.android.internal.os.RuntimeInit$UncaughtHandler and the app crashes immediately as expected.

The only caveat is if for obscure reason the default exception handler is set to null, then the app doesn't crash and no exception is logged. But in this case, uncaught exception in the main thread doesn't crash the app too (but a warning is logged), so maybe we can ignore that case.

@tbruyelle
Copy link
Copy Markdown
Contributor

So for me it's a BIG 👍 !

@DayS
Copy link
Copy Markdown
Contributor

DayS commented Oct 17, 2013

Everything seems to work fine. Great job here 👍

But, while I was testing this PR, I tried to write something like this :

@Click
void buttonCrashableCatchClicked() {
    try {
        crashableBackground();
    } catch (RuntimeException e) {
        Log.d("SANDBOX", "Crashable background crashed correctly");
    }
}

@Background
void crashableBackground() {
    Log.d("SANDBOX", "Crashable background");
    throw new RuntimeException("Boom");
}

Of course, this doesn't work, because the exception is thrown to another thread. But I'm sure we'll have lots of issues about it. So we MUST update the @Background about this.
And for 3.1 we should provide a way to handle exceptions in the class. With a @HandleException annotation for example.

DayS added a commit that referenced this pull request Oct 18, 2013
Fixed #646 sending background exception to the global UncaughtExceptionHandler
@DayS DayS merged commit 53bb943 into androidannotations:develop Oct 18, 2013
@DayS
Copy link
Copy Markdown
Contributor

DayS commented Oct 18, 2013

Thanks for PR 👍
I'll create a new issue about javadoc

@bishopmatthew
Copy link
Copy Markdown

I'm not sure how to use this. In my @background annotated method, I call Thread.setDefaultUncaughtExceptionHandler and pass in my class. But it never seems to be called. Am I misunderstanding?

By the way, I am working off the latest 3.0-SNAPSHOT from the Sonatype repo -- does that have this in it?

@DayS
Copy link
Copy Markdown
Contributor

DayS commented Nov 8, 2013

Could you give us some code ?

I tested again on a sandbox and it works fine. For information, I have something like this :

@EActivity(R.layout.activity_background)
public class BackgroundActivity extends Activity implements UncaughtExceptionHandler {
    @Click
    void buttonCrashableCatchClicked() {
        Thread.setDefaultUncaughtExceptionHandler(this);
        crashableBackground();
    }

    @Override
    public void uncaughtException(Thread thread, Throwable ex) {
        Log.d("SANDBOX", "crashable Background crashed correctly");
    }
}

Note: You don't have to call Thread.setDefaultUncaughtExceptionHandler. It's only if you want to do something special in case of unexpected errors. All other exceptions should be handled IN your @background annotated method?

@bishopmatthew
Copy link
Copy Markdown

Hmm, it seems to be resolved now. I even reverted to the code I was working with yesterday, but it's still working as expected. Only thing I can thing of is to blame it on is IntelliJ using an outdated SNAPSHOT jar or something? I did rebuild the project in between, so maybe that was it.

Thanks for the help!

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants