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

finalize not firing in super gross mode #6250

Closed
benlesh opened this issue Apr 22, 2021 · 0 comments · Fixed by #6251
Closed

finalize not firing in super gross mode #6250

benlesh opened this issue Apr 22, 2021 · 0 comments · Fixed by #6251
Assignees
Labels
bug Confirmed bug

Comments

@benlesh
Copy link
Member

benlesh commented Apr 22, 2021

When using useDeprecatedSynchronousErrorHandling, finalize handlers are not called after a synchronous error is thrown

This is nefarious, because it means that maybe we're not calling any teardowns at all.

import { throwError, config } from "rxjs";
import { finalize } from "rxjs/operators";
console.clear();

config.useDeprecatedSynchronousErrorHandling = true;

try {
  throwError(new Error("test"))
    .pipe(
      finalize(() => {
       // This is never called.
        console.log("finalized");
      })
    )
    .subscribe(() => {
      console.log("nexted");
    });
} catch (err) {
  console.log("here");
}
@benlesh benlesh self-assigned this Apr 22, 2021
@benlesh benlesh added the bug Confirmed bug label Apr 22, 2021
benlesh added a commit to benlesh/rxjs that referenced this issue Apr 22, 2021
Adds tests and ensures a few more scenarios that were hit in Google because they use the deprecated synchronous error handling.

fixes ReactiveX#6250
benlesh added a commit that referenced this issue Apr 23, 2021
…#6251)

* fix: finalize behaves well with useDeprecatedSynchronousErrorHandling

Adds tests and ensures a few more scenarios that were hit in Google because they use the deprecated synchronous error handling.

fixes #6250

* refactor: Move deprecated junk to its own method

Just for readability. The deprecated stuff is a hot mess, and this shows what we get to delete in version 8 more cleanly.

* refactor: Add more comments

* test: Add more tests around gross mode and finalize
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Confirmed bug
Projects
None yet
Development

Successfully merging a pull request may close this issue.

1 participant