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

fix(core): allow controll by signal from fetchOptions #3378

Closed
wants to merge 1 commit into from

Conversation

rexpan
Copy link

@rexpan rexpan commented Sep 12, 2023

Resolves #3377

Summary

The signal passed from fetchOptions by user be ignored and overrided.

Set of changes

Listen to abort event from user passed signal and abort the current request.

@tatianajiselle
Copy link

I can confirm that I am experiencing the bug myself, would appreciate this being resolved.

@kitten
Copy link
Member

kitten commented Sep 12, 2023

@tatianajiselle I left a comment on #3377 trying to answer all three streams of this discussion, but basically, I don't consider this a bug or missing feature right now, and instead, would recommend to write a fetch wrapper and pass that to the Client. There's a further explanation in the issue's comment, but I wouldn't consider using the AbortSignal that's passed in as more useful or more correct, given that it isn't more practical than either a custom exchange or a fetch wrapper, and also isn't a practical public API

Edit: Also, to clarify, I wasn't sure whether the original issue in #3377 and the Discord thread were from the same team, so I assumed as much given the timing 😅 But apologies if they're not

@tatianajiselle
Copy link

all good! moving convo to #3377 for simplicity

@kitten kitten closed this Sep 18, 2023
@kitten
Copy link
Member

kitten commented Sep 18, 2023

As per the conversation in #3377, since signal is not part of the public API (1) because HTTP is not assumed and hence fetchOptions is limited, and (2) since it's not expected to set up a timeout or otherwise pass an AbortSignal in the UI layer to the Client, what this change adds is not expected to work.

@rexpan rexpan deleted the fix-signal branch September 18, 2023 10:39
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.

Signal from fetchOptions not passed to fetch
3 participants