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: emitting unknown error event #3763

Closed
wants to merge 1 commit into from

Conversation

iiAku
Copy link
Contributor

@iiAku iiAku commented Oct 23, 2024

This relates to...

Might fix #3753
related to #3011 as well, since the fix of that one added errorRequest through #3057 and fix another issue

Rationale

This should fixes the following error, but I'm not sure if it's fine console.error this error or if we expect something else. Also I wonder wether we should check in the first place if request exist before accessing it, or this is expected to be catched in that case.

Let me know if we would like to handle that another way.

We are using undici dispatcher with pool client and this error occur.

This is occurring because when the onError from errorRequest in utils is called, then if request is undefined it got caught by the try catch and then we try to emit an error, but it seems emit does not expect such event.

function errorRequest (client, request, err) {
  try {
    request.onError(err)
    assert(request.aborted)
  } catch (err) {
    client.emit('error', err)
  }
}
image
node:events:496
      throw er; // Unhandled 'error' event
      ^
TypeError: Cannot read properties of undefined (reading 'onError')
    at Object.errorRequest (/usr/src/app/node_modules/undici/lib/core/util.js:638:13)
    at ClientHttp2Session.onHTTP2GoAway (/usr/src/app/node_modules/undici/lib/dispatcher/client-h2.js:245:8)
    at ClientHttp2Session.emit (node:events:518:28)
    at Http2Session.onGoawayData (node:internal/http2/core:678:11)
    at Http2Session.callbackTrampoline (node:internal/async_hooks:130:17)
Emitted 'error' event on Client instance at:
    at Object.errorRequest (/usr/src/app/node_modules/undici/lib/core/util.js:641:12)
    at ClientHttp2Session.onHTTP2GoAway (/usr/src/app/node_modules/undici/lib/dispatcher/client-h2.js:245:8)
    [... lines matching original stack trace ...]
    at Http2Session.callbackTrampoline (node:internal/async_hooks:130:17)
Node.js v20.11.0

Undici v6.20.1

Bug Fixes

should close #3753

Breaking Changes and Deprecations

Status

Copy link
Member

@ronag ronag left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This doesn't fix the root issue. Just hides it.

@iiAku
Copy link
Contributor Author

iiAku commented Oct 23, 2024

This doesn't fix the root issue. Just hides it.

I agree and I believe #3761 that one is addressing the root cause.

However, as shown in the screen are we purposely expecting to client.emit('error', err) as the signature does not seems to let doing that. If this is expected in this case, we can close that MR.

@ronag ronag closed this Nov 7, 2024
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.

HTTP/2 GOAWAY event crashes application (Undici v6.20.1)
2 participants