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

Unexpected error when using signal with reason in fetch #49557

Closed
alexkorsun opened this issue Sep 8, 2023 · 9 comments · Fixed by #49559 or nodejs/undici#2597
Closed

Unexpected error when using signal with reason in fetch #49557

alexkorsun opened this issue Sep 8, 2023 · 9 comments · Fixed by #49559 or nodejs/undici#2597
Labels
fetch Issues and PRs related to the Fetch API

Comments

@alexkorsun
Copy link

alexkorsun commented Sep 8, 2023

Version

18.17.1

Platform

Linux undefined 6.2.0-32-generic #32~22.04.1-Ubuntu SMP PREEMPT_DYNAMIC Fri Aug 18 10:40:13 UTC 2 x86_64 x86_64 x86_64 GNU/Linux

Subsystem

fetch

What steps will reproduce the bug?

import express from 'express';

const app = express()
const port = 3333

app.get('/', (req, res) => {
    setTimeout(() => {
        res.send('Hey!')
    }, 10_000)
})

app.listen(port, () => {
    console.log(`App listening on port ${port}`)
})
async function test_signal() {
    try {
        const controller = new AbortController();
        const signal = controller.signal;

        setTimeout(() => {
            const reason = 'heeeey';

            // const reason = {
            //     reason: 'reason'
            // };
            // const reason = undefined;
            controller.abort(reason);
        }, 1_000);

        const res = await fetch('http://localhost:3333/', {signal: signal});

        await res.json()

        console.log('request succeeded > ', res)
    } catch (err) {
        if (err.name === 'AbortError') {
            console.log('request aborted > ', err);
        } else {
            console.log('request failed > ', err)
        }
    }
}

test_signal().catch(err => console.error('final error > ', err));

How often does it reproduce? Is there a required condition?

Always

What is the expected behavior? Why is that the expected behavior?

The expected behavior is to have [AbortError]: This operation was aborted with the reason obtained from the signal.

What do you see instead?

When reason = undefined

request aborted >  DOMException [AbortError]: This operation was aborted
    at Object.fetch (node:internal/deps/undici/undici:11576:11)
    at process.processTicksAndRejections (node:internal/process/task_queues:95:5)
    at async test_signal (/home/alex/WebstormProjects/test/abort-request.js:16:21)

When reason = object

request failed >  { reason: 'reason' }

When reason = string

request failed >  TypeError: invalid_argument
    at Function.captureStackTrace (<anonymous>)
    at Object.fetch (node:internal/deps/undici/undici:11576:11)
    at process.processTicksAndRejections (node:internal/process/task_queues:95:5)
    at async test_signal (/home/alex/WebstormProjects/test/abort-request.js:16:21)

Additional information

No response

@VoltrexKeyva VoltrexKeyva added the fetch Issues and PRs related to the Fetch API label Sep 9, 2023
@zivni
Copy link

zivni commented Sep 12, 2023

This also happens in node v20.6.1

@atlowChemi
Copy link
Member

@alexkorsun the underlaying issue is with https://github.com/nodejs/undici, and the issue here is the reason value is a string, and not an actual error.
This reason is being passed into Error. captureStackTrace which expects an object, and crashes when receiving a string (and any other non-object).

This was already fixed in nodejs/undici#2243, which was released in undici@5.24.0 that did not yet reach Node (a PR for that is open: #49559)

@atlowChemi atlowChemi linked a pull request Sep 13, 2023 that will close this issue
@b5414
Copy link

b5414 commented Nov 10, 2023

node:internal/deps/undici/undici:11372
    Error.captureStackTrace(err, this);
          ^

TypeError: invalid_argument
    at Function.captureStackTrace (<anonymous>)
    at Object.fetch (node:internal/deps/undici/undici:11372:11)

Node.js v20.9.0

@ShadowLNC
Copy link

@atlowChemi Can this issue be reopened please?

I am still seeing this issue in Node 20.10.0 (with Undici 5.26.4, based on the changelog).
I even tried Node 20.8.0 (with Undici 5.24.0 - which was the version you mentioned) and I got the same error.

@atlowChemi
Copy link
Member

@ShadowLNC does the code above reproduce the issue for you?

CC @nodejs/undici

@atlowChemi atlowChemi reopened this Jan 5, 2024
@ShadowLNC
Copy link

@atlowChemi Using the code from the original issue description, I do indeed get the TypeError as described.

For my other testsing I was using this one-line script fetch('https://example.com', {signal: AbortSignal.abort("fake error")}); - output from the REPL is as follows (showing undefined reason as well as string value)

> fetch('https://example.com', {signal: AbortSignal.abort()});
Promise {
  <pending>,
  [Symbol(async_id_symbol)]: 1082,
  [Symbol(trigger_async_id_symbol)]: 6
}
> Uncaught:
DOMException [AbortError]: This operation was aborted
    at Object.fetch (node:internal/deps/undici/undici:11730:11)
    at process.processTicksAndRejections (node:internal/process/task_queues:95:5)


> fetch('https://example.com', {signal: AbortSignal.abort("fake error")});
Promise {
  <pending>,
  [Symbol(async_id_symbol)]: 1139,
  [Symbol(trigger_async_id_symbol)]: 6
}
> Uncaught TypeError: invalid_argument
    at Function.captureStackTrace (<anonymous>)
    at Object.fetch (node:internal/deps/undici/undici:11730:11)
    at process.processTicksAndRejections (node:internal/process/task_queues:95:5)
> 

@atlowChemi
Copy link
Member

@garazy
Copy link

garazy commented Apr 16, 2024

I came across this as I had the same issue even with v21.7.31. I fixed it with new DOMException ( 'your reason' ) as the parameter to the .abort method of AbortController. This stopped this error from happening for me https://developer.mozilla.org/en-US/docs/Web/API/DOMException

Causes the undici error - controller.abort()
Doesn't cause the error - controller.abort(new DOMException('reason'))

Hope this helps anyone with similar issue.

@equt
Copy link

equt commented Jul 4, 2024

FYI, the fix in undici was released in https://github.com/nodejs/undici/releases/tag/v6.3.0, and is available in Node in v22.0.0.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
fetch Issues and PRs related to the Fetch API
Projects
None yet
8 participants