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

Support using AbortSignal to abort requests #2479

Open
wants to merge 4 commits into
base: beta
Choose a base branch
from

Conversation

paulbrimicombe
Copy link

See #2478

Problem

  • HTTP requests can now be aborted using an AbortSignal
  • nock currently has no support for this

Solution

  • Destroy the request when an abort signal enters the aborted state

@paulbrimicombe
Copy link
Author

Any chance that this could be reviewed / merged please? It's causing us issues when testing code that uses AbortSignal for timeouts. If there are reasons why this can't be done, etc., please let me know so I can think about solutions.

@gr2m
Copy link
Member

gr2m commented Jan 19, 2024

Sorry that the current nock version still supports Node 10 😞 We are working on support for the native fetch as our priority, we will probably bump the minimal node version to 18 as part of that

@paulbrimicombe
Copy link
Author

Sorry that the current nock version still supports Node 10 😞 We are working on support for the native fetch as our priority, we will probably bump the minimal node version to 18 as part of that

Hi @gr2m , thanks so much for getting back to me! I'm pretty sure it's only the tests that don't run on earlier versions of Node but I quite understand if you want to hang fire on this until the native fetch stuff is complete.

@schrizsz
Copy link

schrizsz commented Jan 29, 2024

Had to use this fork to test a request deduplication fn using abortController code in jest tests for FE 🙏 to merge. (node 18 + jest unit test)

@mikicho
Copy link
Contributor

mikicho commented Feb 16, 2024

Maybe we can merge this for the beta instead.

@gr2m gr2m changed the base branch from main to beta February 17, 2024 18:07
@gr2m
Copy link
Member

gr2m commented Feb 17, 2024

I changed head and updated the PR with latest changes from the beta branch

@bkendall
Copy link

(this looks awesome - came here looking to see why my timeouts weren't working on the beta branch, and this would do it!)

@mikicho
Copy link
Contributor

mikicho commented Apr 29, 2024

@bkendall If you are referring to the fetch API, please see this URL: #2602.

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.

None yet

5 participants