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

fetch is sometimes crashing entire node process since upgrading from node 22.9.0 to 23.1.0 #3813

Open
KMatuszak opened this issue Nov 8, 2024 · 8 comments

Comments

@KMatuszak
Copy link

Bug Description

fetch is sometimes crashing entire node process since upgrading from node 22.9.0 to 23.1.0

Logs & Screenshots

node:events:485
      throw er; // Unhandled 'error' event
      ^

TypeError: terminated
    at Fetch.onAborted (node:internal/deps/undici/undici:11020:53)
    at Fetch.emit (node:events:507:28)
    at Fetch.terminate (node:internal/deps/undici/undici:10178:14)
    at Object.onError (node:internal/deps/undici/undici:11141:38)
    at Request.onError (node:internal/deps/undici/undici:2094:31)
    at Object.errorRequest (node:internal/deps/undici/undici:1591:17)
    at TLSSocket.<anonymous> (node:internal/deps/undici/undici:6263:16)
    at TLSSocket.emit (node:events:519:35)
    at node:net:350:12
    at TCP.done (node:_tls_wrap:650:7)
Emitted 'error' event on Readable instance at:
    at emitErrorNT (node:internal/streams/destroy:170:8)
    at emitErrorCloseNT (node:internal/streams/destroy:129:3)
    at process.processTicksAndRejections (node:internal/process/task_queues:90:21) {
  [cause]: Error: read ECONNRESET
      at TLSWrap.onStreamRead (node:internal/stream_base_commons:216:20) {
    errno: -104,
    code: 'ECONNRESET',
    syscall: 'read'
  }
}

Environment

amd64 node:23.1.0-bookworm docker image

@KMatuszak KMatuszak added the bug Something isn't working label Nov 8, 2024
@mcollina
Copy link
Member

mcollina commented Nov 8, 2024

Thanks for reporting!

Can you provide steps to reproduce? We often need a reproducible example, e.g. some code that allows someone else to recreate your problem by just copying and pasting it. If it involves more than a couple of different file, create a new repository on GitHub and add a link to that.

@KMatuszak
Copy link
Author

Unfortunately I don't think it's possible to create this, and I know that this report probably doesn't help much without it. It happens randomly about once a day when doing various simple fetches.

@koplenov
Copy link

koplenov commented Nov 8, 2024

+1

@mcollina mcollina added missing minimal reproduction and removed bug Something isn't working labels Nov 9, 2024
@artur-ma
Copy link
Contributor

artur-ma commented Nov 28, 2024

I dont think it is related to node version, emitting "error" event has a special meaning in nodjes, same as throwing exception

and if u are not handling it, it will just crash the process.

This is the behaviour both for 22.x, 23.x and older version of nodejs

by adding

process.on('uncaughtException', err => console.error(`Uncaught Exception`, { err }));

should avoid process crash

@Uzlopak
Copy link
Contributor

Uzlopak commented Nov 28, 2024

is it really uncaughtException? Thought it is uncaughtPromiseRejection?

@artur-ma
Copy link
Contributor

artur-ma commented Nov 28, 2024

is it really uncaughtException? Thought it is uncaughtPromiseRejection?

Im sure you know better than me, since u are nodejs contributor 😄
But from my observation, it is uncaughtException


on second thought, it is possible that there was some change in 23.x, specifically in streams, that emits error without handling it

But anyway I do not think it is undici issue

@KMatuszak
Copy link
Author

KMatuszak commented Nov 28, 2024

All my fetch calls are done in try block. I’m not sure but if that’s issue on my end, shouldn’t the file be mentioned in the crash stack trace?

I didn’t said this is caused by node version, when node was updated from 22.9.0 to 23.1.0 also undici version bundled with node has changed, so that’s probably the issue. This has never occurred up to version 22.9.0, but now it still happens even on 23.3.0

@artur-ma
Copy link
Contributor

artur-ma commented Dec 2, 2024

It feels similar to this one
nodejs/node#45244
and also this one
nodejs/node#54670

People complain that tls is initialized internally without handling the "error" event which causes process to crash on ECONNRESET, and there is no way to handle from outside

I would recommend you to open the issue on node repo

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

5 participants