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

Let exceptions crash the app #55

Closed
fmweigl opened this issue Jan 18, 2014 · 13 comments
Closed

Let exceptions crash the app #55

fmweigl opened this issue Jan 18, 2014 · 13 comments
Labels
confirmed enhancement New feature or request
Milestone

Comments

@fmweigl
Copy link

fmweigl commented Jan 18, 2014

Hi, if I have an Exception that would normally crash the app within the onEvent callback method the app doesn't crash and the exception is logged by EventBus.
Is there a way to change that behaviour to the default one (exception -> crash) ?

@doridori
Copy link

I guess it would be good (if you cant already) to register an optional exception handler and the default one would NOT catch and log the exception as it does currently. I find with a few libs i use when the exception is sucked up anywhere it can occasionally lead to wasted debugging time.

@benoitdion
Copy link

+1

@danielbenzvi
Copy link

You can always catch the exception using the special exception event and re-throw it from a runnable posted to the main thread's Handler.

@doridori
Copy link

thats useful to know - I would think it would make sense to have this as default behaviour for the sake of newcomers - I really dislike it when a lib sucks up exceptions by default from proir time spent debugging issues with other libs!

@fmweigl
Copy link
Author

fmweigl commented Jan 22, 2014

@danielbenzvi Do I get this right, there's an event for exceptions within onEvent methods, I can register for this event like for any other event and then throw a RuntimeException myself?

@danielbenzvi
Copy link

@asco33 correct. You can listen for the SubscriberExceptionEvent. You have access to the causing event, causing subscriber and the offending throwable.
you can just post a Runnable and re-throw the Exception as if it was thrown from the main thread.

@yarian
Copy link

yarian commented Mar 20, 2014

+1

Thanks for the work-around danielbenzvi!

@electrolobzik
Copy link

If EventBus will not handle exceptions in onEvent methods, they will be thrown in unpredictable and improper places. If subscriber is registered at the moment of posting event, exception will be thrown by post method (but it will be exception in subscriber!). Otherwise it will be thrown by registerSticky method, which is better but still unhandy. The way it works now is quite flexible, as you can handle exception exactly at the place it is thrown and do whatever you want, for example throwing runtime exception, as @danielbenzvi said above. One thing is that maybe @greenrobot should put detailed description for this into separate chapter in readme, as I suppose it can help many users.

@intrepidclass
Copy link

For anyone wondering what the code for that workaround would look like:

public void onEvent(final SubscriberExceptionEvent _event) {
    // Crash the app cause it would have crashed if EventBus hadn't put a stop to it.
    // We want to know about exceptions so we can fix them.
    new Handler().post(new Runnable() {
        @Override
        public void run() {
            Crashlytics.log("Crash in onEvent() in " + _event.causingSubscriber.getClass().getSimpleName() + " while handling " + _event.causingEvent.getClass().getSimpleName());
            throw new RuntimeException(_event.throwable);
        }
    });
}

@greenrobot
Copy link
Owner

#124

@greenrobot greenrobot added confirmed enhancement New feature or request labels Nov 8, 2014
@greenrobot
Copy link
Owner

See #124 - Use builder.failFast(true) to enable.

@greenrobot greenrobot added this to the 2.3.0 milestone Nov 9, 2014
@fmweigl
Copy link
Author

fmweigl commented Jan 21, 2015

I cannot find the failFast method on the EventBus builder in version 2.4.0. Ist this implemented yet? If not, for what version is it planned?

@greenrobot
Copy link
Owner

It got renamed to throwSubscriberException

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
confirmed enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

8 participants