fix(core): remove rejectErrors option encourages uncaught exceptions#60397
fix(core): remove rejectErrors option encourages uncaught exceptions#60397atscott wants to merge 1 commit intoangular:mainfrom
rejectErrors option encourages uncaught exceptions#60397Conversation
This commit removes the previously added `rejectErrors` option from `toSignal` which was meant to create similar behavior to what happens with unhandled errors in `AsyncPipe`. This option promotes unhandled exceptions and attaches behaviors that we do not believe are desirable as an option offered by the framework: * Unhandled errors that are thrown lose the context of where the error ocurred. Throwing the error where the signal is read allows error handling to be performed at the signal's usage location * With this feature, the value of the signal remains set to the initial value or the previous successful value coming out of the `Observable`. We do not feel this is appropriate implicit behavior but should be an explicit choice by the application. Signals are built to represent state. When an observable stream is converted to a stateful representation, there should be a choice made about what state should be presented when an error occurs * If an error occurs but the signal value is never read in its error state, this is not an application state error that should result in an unhandled exception. * While Angular does not have error boundaries today, there is likely to be investigation in the near future to address increased risk of errors thrown from signals such as this in templates. Without pre-designing any specifics, it is possible that this type of feature would require the error to be thrown from the signal. We are subsequently not prepared to commit to stabilizing the `toSignal` API with the `rejectErrors` option and its current behavior. Developers that desire similar behavior to `rejectErrors` have several options, the simplest of which would be something similar to `toSignal(myStream.pipe(catchError(() => EMPTY)))`.
70e15b5 to
d43c763
Compare
pkozlowski-opensource
left a comment
There was a problem hiding this comment.
LGTM
Reviewed-for: public-api
devversion
left a comment
There was a problem hiding this comment.
Should we note this as a breaking change? (or do I miss something?)
it was seemingly added in v17 via 7bb3ffb
|
@devversion sure, I can add it for good measure. There were only two uses internally and API is still in dev preview so technically doesn’t require it. I also felt like because it’d break at compile time rather than a behavioral change you might not otherwise notice, it’d be obvious enough. |
|
yeah, I was under impression that typically we still mark those as breaking changes (since they are breaking); but we are just allowed to make them for dev-preview code even in e.g. a minor |
|
This PR was merged into the repository by commit 48974c3. The changes were merged into the following branches: main |
|
This issue has been automatically locked due to inactivity. Read more about our automatic conversation locking policy. This action has been performed automatically by a bot. |
This commit removes the previously added
rejectErrorsoption fromtoSignalwhich was meant to create similar behavior to what happens with unhandled errors inAsyncPipe.This option promotes unhandled exceptions and attaches behaviors that we do not believe are desirable as an option offered by the framework:
Observable. We do not feel this is appropriate implicit behavior but should be an explicit choice by the application. Signals are built to represent state. When an observable stream is converted to a stateful representation, there should be a choice made about what state should be presented when an error occurstoSignalAPI with therejectErrorsoption and its current behavior.Developers that desire similar behavior to
rejectErrorshave several options, the simplest of which would be something similar totoSignal(myStream.pipe(catchError(() => EMPTY))).