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

feat(ErrorHandler): use the angular ErrorHandler for reporting errors #667

Merged
merged 1 commit into from
Jan 9, 2018

Conversation

jbedard
Copy link
Contributor

@jbedard jbedard commented Dec 23, 2017

Fixes #626

This is a v5 alternative to #635. Can potentially try to keep InvalidActionError and EffectError but I'm not sure if that is a standard thing to do with ErrorHandler?

BREAKING CHANGE:

Errors are now reported to the @angular/core ErrorHandler.
Errors are no longer reported to the console.

@coveralls
Copy link

Coverage Status

Coverage increased (+0.3%) to 92.959% when pulling 9eb1402 on jbedard:626-drop-console into 95dd2dc on ngrx:master.

@brandonroberts
Copy link
Member

The ErrorHandler reports to the console by default, so it wouldn't be a breaking change.

@jbedard
Copy link
Contributor Author

jbedard commented Jan 3, 2018

It is a breaking change if #635 gets released in v4. Shall I remove the breaking change from the commit message though?

@brandonroberts
Copy link
Member

brandonroberts commented Jan 6, 2018

@jbedard The breaking change should remain, but the format should be altered so it shows in the changelog correctly..

BREAKING CHANGE: Change description here

BEFORE:

Behavior before change.

AFTER:

Behavior after change.

@brandonroberts brandonroberts self-assigned this Jan 7, 2018
@brandonroberts
Copy link
Member

@jbedard this one is ready to land once the commit message is updated.

Fixes ngrx#626

BREAKING CHANGE: the ErrorReporter has been replaced with ErrorHandler
from angular/core.

BEFORE:

Errors were reported to the ngrx/effects ErrorReporter. The
ErrorReporter would log to the console by default.

AFTER:

Errors are now reported to the @angular/core ErrorHandler.
@coveralls
Copy link

Coverage Status

Coverage increased (+0.3%) to 93.03% when pulling 66973d1 on jbedard:626-drop-console into 77c832a on ngrx:master.

@jbedard
Copy link
Contributor Author

jbedard commented Jan 9, 2018

I've updated the commit message, and rebased. Let me know if the commit message is ok...

@coveralls
Copy link

Coverage Status

Coverage increased (+0.3%) to 93.03% when pulling 23b98b8 on jbedard:626-drop-console into 77c832a on ngrx:master.

@brandonroberts
Copy link
Member

LGTM

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

Successfully merging this pull request may close these issues.

3 participants