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

Trigger an E_USER_ERROR instead of throwing an exception from done() #97

Merged
merged 1 commit into from
Sep 19, 2017

Conversation

jsor
Copy link
Member

@jsor jsor commented Aug 24, 2017

If an error (either a thrown exception or returned rejection) escapes the done() callbacks, it will now cause a fatal error by using trigger_error() with E_USER_ERROR instead of (re)throwing.

Because promise resolution is synchronous, those exceptions bubbled up to the
reject() call which mixed resolution and consumption parts.

Closes #88

@jsor jsor added this to the v3.0 milestone Aug 24, 2017
@jsor jsor requested review from WyriHaximus and clue August 24, 2017 19:57
If an error (either a thrown exception or returned rejection) escapes the
done() callbacks, it will now cause a fatal error by using trigger_error() with
E_USER_ERROR instead of (re)throwing.

Because promise resolution is synchronous, those exceptions bubbled up to the
reject() call which mixed resolution and consumption parts.
Copy link

@kelunik kelunik left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM.

@jsor jsor merged commit 9dcffcc into reactphp:master Sep 19, 2017
@jsor jsor deleted the done-error branch September 19, 2017 13:53
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

done() exceptions bubble up into the wrong place
4 participants