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

fromFetch cannot abort all responses #4744

Closed
cartant opened this issue Apr 27, 2019 · 7 comments
Closed

fromFetch cannot abort all responses #4744

cartant opened this issue Apr 27, 2019 · 7 comments
Labels
feature PRs and issues for features

Comments

@cartant
Copy link
Collaborator

cartant commented Apr 27, 2019

Bug Report Feature Request

Current Behavior

fromFetch returns an observable that emits the request's Response object. When json() or text() is called on the Response, a promise is returned. My understanding is that the AbortController used in the fetch call also aborts promises returned by the aforementioned methods.

In #4742, a bug was fixed: fromFetch was calling abort as part of the implicit unsubscribe that occurs after completion. This would abort any still pending responses. When fetch resolves, a flag is now set to prevent abort being called in the unsubscribe teardown.

However, that means there is no way of aborting the promises returned by calls to Response methods.

Possible Solution

Add another signature to fromFetch so that a selector can be passed:

export function fromFetch<R>(
  input: string | Request,
  init: RequestInit | undefined,
  selector: (response: Response) => Observable<R>,
): Observable<R>;

Much like the selector passed to multicast, observables composed within the selector will be 'contained' within the observable returned by fromFetch and unsubscribing from said observable will facilitate the aborting of any promises returned by Response methods within the selector.

If a selector is not passed, fromFetch should continue to behave as in #4742 and not abort upon unsubscription if the promise returned by fetch has resolved.

Additional Information

Note that the changes introduced in https://github.com/ReactiveX/rxjs/pull/3963/files will ensure that unsubscription from the observable returned by fromFetch will occur ASAP if said observable serves as the source for, say, concatMap. So extending the 'life' of the AbortController for an emitted Response - if the selector is not used - would be a non-trivial undertaking.

@cartant cartant added the bug Confirmed bug label Apr 27, 2019
@benlesh
Copy link
Member

benlesh commented May 11, 2019

This is resolved in 6.5.2

@benlesh benlesh closed this as completed May 11, 2019
@cartant
Copy link
Collaborator Author

cartant commented May 11, 2019

Nope. This isn't resolved. With the current implementation, it's not possible to abort responses that use the chunked transfer encoding or another mechanism that results in only a partial body being available at the time fetch resolves and returns the Response object. In those cases, the promise returned by json(), for example, won't resolve until the entire body is available. And, with the current implementation, the latter promise cannot be aborted.

@cartant cartant reopened this May 11, 2019
@cartant cartant added feature PRs and issues for features and removed bug Confirmed bug labels May 11, 2019
@ReactiveX ReactiveX deleted a comment from kambing86 Jan 21, 2020
@ReactiveX ReactiveX deleted a comment from kambing86 Jan 21, 2020
@ReactiveX ReactiveX deleted a comment from kambing86 Jan 21, 2020
@ReactiveX ReactiveX deleted a comment from kambing86 Jan 21, 2020
@ReactiveX ReactiveX deleted a comment from kambing86 Jan 21, 2020
cartant added a commit to cartant/rxjs that referenced this issue Feb 9, 2020
cartant added a commit to cartant/rxjs that referenced this issue Mar 19, 2020
cartant added a commit to cartant/rxjs that referenced this issue Apr 3, 2020
cartant added a commit to cartant/rxjs that referenced this issue Apr 19, 2020
@curiousercreative
Copy link

@cartant should the canceling of a chunked response via fromFetch be working? Perhaps a new issue is warranted, but I'm observing the following (v6.5.5):

  1. If the fetch in question has no received any body, unsubscribing from the observable will cancel the fetch request properly (hooray!)
  2. If the fetch has received any body, then the fetch won't be canceled when observable is unsubscribed.

@cartant
Copy link
Collaborator Author

cartant commented May 27, 2020

@curiousercreative Nope. The PR with the selector - a selector has to be passed in the InitRequest options for responses with content that is received after the headers - wasn't merged until May 19 and 6.5.5 was published on April 3. So this functionality won't be available until 6.5.6 - @benlesh.

@curiousercreative
Copy link

@cartant hey, that's great news! I'll be looking forward to the next release. Thanks for adding this feature.

@cartant
Copy link
Collaborator Author

cartant commented May 27, 2020

@curiousercreative BTW, if you are interested, the reason for the selector is explained here: https://ncjamieson.com/understanding-fromfetch/

@curiousercreative
Copy link

@cartant thanks for that follow-up. My implementation actually doesn't have to wait for the next release. In my case, I'm streaming my chunked response using response.body.getReader() and after reading through your article and this MDN reference, I found that I can simply cancel the reader stream when my custom Observable unsubscribes (from a switchMap). Works like a charm, but I wouldn't have gotten it there without your help!

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

No branches or pull requests

3 participants