Skip to content

fix: include AggregateError causes in request failure reason - #1914

Closed
dyk1454683243-sudo wants to merge 1 commit into
node-fetch:mainfrom
dyk1454683243-sudo:fix/aggregate-error-reason-1841
Closed

dyk1454683243-sudo wants to merge 1 commit into
node-fetch:mainfrom
dyk1454683243-sudo:fix/aggregate-error-reason-1841

Conversation

@dyk1454683243-sudo

Copy link
Copy Markdown

Purpose

When Node's https/net stack fails both IPv4 and IPv6 connect attempts, it emits an AggregateError whose message is empty. node-fetch only interpolated error.message, so users saw:

FetchError: request to https://example.com/ failed, reason:

Fixes #1841

Changes

  • When formatting the request-failure reason, if error.message is empty, join nested error.errors codes/messages with and.
  • Single Error instances with a non-empty message are unchanged.
  • Added a unit test that injects an AggregateError (or empty-message multi-error on Node < 15) on the request error path via a custom http.Agent.

Additional information

Dual-stack connect failures are the usual source of this AggregateError (see Node net.js).

  • I updated readme
  • I added unit test(s)

When Node's dual-stack connect fails, https/net emits AggregateError
with an empty message. Use nested error codes/messages so FetchError
no longer ends with an empty "reason:".

Co-authored-by: David <dyk1454683243-sudo@users.noreply.github.com>
@dyk1454683243-sudo

Copy link
Copy Markdown
Author

Withdrawing this PR while I clean up a high-volume open-PR backlog. Sorry for the noise — happy to come back later with a focused change if useful.

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.

Failing without clear reason if https module throwed AggregateError

2 participants