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

Error thrown by rxjs observer not always sent to ErrorHandler.handleError #14316

Closed
christianacca opened this issue Feb 5, 2017 · 19 comments · Fixed by angular/zone.js#884 or #15208
Closed

Comments

@christianacca
Copy link

christianacca commented Feb 5, 2017

I'm submitting a ... (check one with "x")

[x] bug report => search github for a similar issue or PR before submitting

Current behavior
Error thrown in an observable is not sent to the ErrorHandler.handleError method when observeOn(Scheduler.asap) is used.

Take the following example that DOES result in an error being sent:

    const ticks$ = Observable.interval(1000).take(10);
    
    this.mapped$ = ticks$.map(value => {
      if (value === 3) {
        throw new Error('oops');
      }
      return { value };
    });
<h4>Mapped ticks from an observable... {{mapped$ | async | json}}</h4>

The following example that does NOT result in an error being sent:

    const ticks$ = Observable.interval(1000).take(10)
      .observeOn(Scheduler.asap);
      
    this.mapped$ = ticks$.map(value => {
      if (value === 3) {
        throw new Error('oops');
      }
      return { value };
    });

Expected behavior

An error thrown by an observer scheduled with Scheduler.asap is sent to ErrorHandler.handleError

Minimal reproduction of the problem with instructions*

I've reproduced the problem in a new angular seed project generated by the latest vs of angular-cli (1.0.0-beta.30).

To run:

Notice that the error thrown by the map operator is logged to the console, but not sent to DelegatingErrorHandler.handleError (a custom ErrorHandler)

What is the motivation / use case for changing the behavior?

Using observeOn(Scheduler.asap) is a workaround to a problem with rxjs as documented here:

https://medium.com/@benlesh/on-the-subject-of-subjects-in-rxjs-2b08b7198b93#3114

Please tell us about your environment:

  • Windows 10

  • Visual Studio code

  • Angular version: 2.4.6

  • Browser: [all | Chrome XX | Firefox XX | IE XX | Safari XX | Mobile Chrome XX | Android X.X Web Browser | iOS XX Safari | iOS XX UIWebView | iOS XX WKWebView ]

Only tried in Chrome 55.0.2883.87

  • Language: [all | TypeScript X.X | ES6/7 | ES5]

TypeScript 2.0.10

@christianacca
Copy link
Author

That's what I said I tried to do:

I tried getting a plunker working with all the Rxjs pieces but after an hour I gave up

Here's as far as I got: http://plnkr.co/edit/ldm5hGVjnvacRFpkIRXv?p=preview

It seems the ES2015 import's that I've added for the Rxjs modules are not correct.

Do you have an example plunker that showcases using Rxjs scheduler's with angular?

Thanks

@christianacca
Copy link
Author

OK so I have got the plunker working with the correct imports: http://plnkr.co/edit/ldm5hGVjnvacRFpkIRXv?p=preview

And the problem is not reproducible in this.

To try and cut the problem space down, I'll create another git repo with a minimum angular seed project generated by the angular-cli with the same exact same versions of angular as the plunker (2.4.6 vs 2.4.1).

If this reproduces the problem, then there is going to be more material to work on.

christianacca pushed a commit to christianacca/rxjs-err-async-pipe that referenced this issue Feb 6, 2017
christianacca pushed a commit to christianacca/rxjs-err-async-pipe that referenced this issue Feb 6, 2017
christianacca pushed a commit to christianacca/rxjs-err-async-pipe that referenced this issue Feb 6, 2017
@christianacca
Copy link
Author

christianacca commented Feb 6, 2017

I've reproduced the problem in a new angular seed project generated by the latest vs of angular-cli (1.0.0-beta.30).

See https://github.com/christianacca/rxjs-err-async-pipe

Note: the versions of the dependencies in this new project are identical to the plunker version that does work.

So it appears a problem with Webpack build (via the cli) of angular and it's rxjs dependency.

Reproducing problem

For convenience here is the steps to reproduce the problem with this new project:

Notice that the error thrown by the map operator is logged to the console, but not to sent to DelegatingErrorHandler.handleError (a custom ErrorHandler)

Remove .observeOn(Scheduler.asap), wait for the browser to refresh, and now you'll see the error alert'ed by DelegatingErrorHandler

@DzmitryShylovich
Copy link
Contributor

@christianacca can you pls update the first post and remove unnecessary comments? A lot of different links, it's hard to understand what actually the problem is. Thx.

@christianacca
Copy link
Author

I've updated my original post...

@DzmitryShylovich
Copy link
Contributor

 ngZone.onError.subscribe({next: (error: any) => { exceptionHandler.handleError(error); }});

this is how error handler receives errors. Looks like it's zonejs issue.

@DzmitryShylovich
Copy link
Contributor

DzmitryShylovich commented Feb 8, 2017

Probably will be fixed here ReactiveX/rxjs#2266

There's nothing we can do about it inside angular

@christianacca
Copy link
Author

Yeah that was my guess.

@christianacca
Copy link
Author

Although if this was the problem, why would the plunker example with the same code work?

mhevery added a commit to mhevery/angular that referenced this issue Mar 16, 2017
…ar#15077)

ErrorHandler can not throw errors because it will unsubscribe itself from
the error stream.

Zones captures errors and feed it into NgZone, which than has a Rx Observable
to feed it into ErrorHandler. If the ErroHandler throws, then Rx will teardown
the observable which in essence causes the ErrorHandler to be removed from the
error handling. This implies that the ErrorHandler can never throw errors.

Closes angular#14949
Closes angular#15182
Closes angular#14316
mhevery added a commit to mhevery/angular that referenced this issue Mar 16, 2017
…ar#15077)

ErrorHandler can not throw errors because it will unsubscribe itself from
the error stream.

Zones captures errors and feed it into NgZone, which than has a Rx Observable
to feed it into ErrorHandler. If the ErroHandler throws, then Rx will teardown
the observable which in essence causes the ErrorHandler to be removed from the
error handling. This implies that the ErrorHandler can never throw errors.

Closes angular#14949
Closes angular#15182
Closes angular#14316
mhevery added a commit to mhevery/angular that referenced this issue Mar 16, 2017
…ar#15077)

ErrorHandler can not throw errors because it will unsubscribe itself from
the error stream.

Zones captures errors and feed it into NgZone, which than has a Rx Observable
to feed it into ErrorHandler. If the ErroHandler throws, then Rx will teardown
the observable which in essence causes the ErrorHandler to be removed from the
error handling. This implies that the ErrorHandler can never throw errors.

Closes angular#14949
Closes angular#15182
Closes angular#14316
chuckjaz pushed a commit that referenced this issue Mar 16, 2017
… (#15208)

ErrorHandler can not throw errors because it will unsubscribe itself from
the error stream.

Zones captures errors and feed it into NgZone, which than has a Rx Observable
to feed it into ErrorHandler. If the ErroHandler throws, then Rx will teardown
the observable which in essence causes the ErrorHandler to be removed from the
error handling. This implies that the ErrorHandler can never throw errors.

Closes #14949
Closes #15182
Closes #14316
SamVerschueren pushed a commit to SamVerschueren/angular that referenced this issue Mar 18, 2017
…ar#15077) (angular#15208)

ErrorHandler can not throw errors because it will unsubscribe itself from
the error stream.

Zones captures errors and feed it into NgZone, which than has a Rx Observable
to feed it into ErrorHandler. If the ErroHandler throws, then Rx will teardown
the observable which in essence causes the ErrorHandler to be removed from the
error handling. This implies that the ErrorHandler can never throw errors.

Closes angular#14949
Closes angular#15182
Closes angular#14316
@christianacca
Copy link
Author

Unfortunately this issue is not closed by PR #15208 - please can you therefore reopen.

I have tested against angular 4.1.0 and still see the exact same behaviour as I originally reported.

To reproduce with angular 4.1.0 please follow the steps outlined here: #14316 (comment)

@matsko matsko reopened this May 6, 2017
@matsko matsko added the area: core Issues related to the framework runtime label May 6, 2017
@tbosch tbosch added area: zones and removed area: core Issues related to the framework runtime labels May 8, 2017
@christianacca
Copy link
Author

christianacca commented May 14, 2017

More information:

  • The problem can be reproduced in angular 4.1.2 + zone.js 0.8.10
  • The problem does not occur when the component demonstrating the issue is lazy loaded

An error thrown by an observer scheduled with Scheduler.asap is sent to ErrorHandler.handleError

@ccrowhurstram
Copy link

ccrowhurstram commented Jul 31, 2017

FYI, the problem can be reproduced in angular 4.3.2 + zone.js 0.8.16.

This is surprising given that this version of zone.js lands angular/zone.js#843 that was hoping to resolve this problem...

Notes:

asnowwolf pushed a commit to asnowwolf/angular that referenced this issue Aug 11, 2017
…ar#15077) (angular#15208)

ErrorHandler can not throw errors because it will unsubscribe itself from
the error stream.

Zones captures errors and feed it into NgZone, which than has a Rx Observable
to feed it into ErrorHandler. If the ErroHandler throws, then Rx will teardown
the observable which in essence causes the ErrorHandler to be removed from the
error handling. This implies that the ErrorHandler can never throw errors.

Closes angular#14949
Closes angular#15182
Closes angular#14316
@JiaLiPassion
Copy link
Contributor

JiaLiPassion commented Aug 22, 2017

@ccrowhurstram , thank you for posting the issue, current angular/zone.js#843 patch doesn't fully fix your issue, I have made a new PR (angular/zone.js#884) to fix this one, I have test my idea and it should work.

@ccrowhurstram
Copy link

Excellent @JiaLiPassion.

Thanks for digging into this - saves me from having to spam this issue with each angular release ;-)

@bryanrideshark
Copy link

@ccrowhurstram, you aren't the only one hitting this - Glad to see that it's getting looked at!

@JiaLiPassion
Copy link
Contributor

please wait for the next release of zone.js, and I will post how to load the new rxjs patch.

@JiaLiPassion
Copy link
Contributor

@ccrowhurstram , the new zone.js have been released, and I test it with your repo, it worked!

to load the patch,

  • npm install zone.js@0.8.17
  • add import 'zone.js/dist/zone-patch-rxjs'; into your app.module.ts

@christianacca
Copy link
Author

Brilliant @JiaLiPassion :-)

juleskremer pushed a commit to juleskremer/angular that referenced this issue Aug 28, 2017
…ar#15077) (angular#15208)

ErrorHandler can not throw errors because it will unsubscribe itself from
the error stream.

Zones captures errors and feed it into NgZone, which than has a Rx Observable
to feed it into ErrorHandler. If the ErroHandler throws, then Rx will teardown
the observable which in essence causes the ErrorHandler to be removed from the
error handling. This implies that the ErrorHandler can never throw errors.

Closes angular#14949
Closes angular#15182
Closes angular#14316
@angular-automatic-lock-bot
Copy link

This issue has been automatically locked due to inactivity.
Please file a new issue if you are encountering a similar or related problem.

Read more about our automatic conversation locking policy.

This action has been performed automatically by a bot.

@angular-automatic-lock-bot angular-automatic-lock-bot bot locked and limited conversation to collaborators Sep 12, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
7 participants