Skip to content

fix: Surface meaningful error messages for connection failures#122

Merged
MattDevy merged 5 commits into
mainfrom
MattDevy/fix/issue-99
Apr 13, 2026
Merged

fix: Surface meaningful error messages for connection failures#122
MattDevy merged 5 commits into
mainfrom
MattDevy/fix/issue-99

Conversation

@MattDevy

Copy link
Copy Markdown
Contributor

Summary

  • ConnectionError now returns connection_error code with the target URL appended to the message (e.g., connection failed (http://localhost:19999/)) instead of an empty transport_error message
  • TimeoutError now returns timeout_error code with the original timeout message preserved
  • Generic transport errors remain as transport_error for any other error types

Test plan

  • 6 new regression tests covering: non-empty message, empty message with URL from meta, both message and URL, empty fallback, timeout errors, generic transport errors
  • Full test suite passes (513/513)
  • Lint clean
  • Build clean

Closes #99

…lures

ConnectionError and TimeoutError from @elastic/transport now produce
distinct error codes (connection_error, timeout_error) with descriptive
messages including the target URL when available, instead of the generic
transport_error with an empty message.

Closes #99
@MattDevy MattDevy requested a review from JoshMock April 13, 2026 11:00
@MattDevy

MattDevy commented Apr 13, 2026

Copy link
Copy Markdown
Contributor Author

This fix works around two upstream issues in @elastic/transport that I've reported:

If those are addressed upstream, the connectionMessage() helper here could be simplified (or removed) since the library would provide meaningful messages directly.

Comment thread src/es/handler.ts Outdated
Comment thread src/es/handler.ts Outdated
@JoshMock JoshMock enabled auto-merge (squash) April 13, 2026 16:38
@MattDevy MattDevy disabled auto-merge April 13, 2026 16:54
… module

Adapts to the refactor that extracted error helpers into src/es/errors.ts.
Removes stale local copies from handler.ts and moves the ConnectionError
and TimeoutError branches into the shared module.

Reverts `??` back to `||` for message fallbacks: empty string is the bug
scenario (elastic/elastic-transport-js#366), so we need falsy checks, not
just nullish checks.
@MattDevy

MattDevy commented Apr 13, 2026

Copy link
Copy Markdown
Contributor Author

Hey @JoshMock, a small note on the ?? to || revert in the latest commit: the empty string case is the actual bug we're fixing here. The transport library surfaces ConnectionError with message: "" for socket failures, so we need || to catch that and fall back to 'connection failed'. With ??, the empty string passes through since it's not nullish, which re-introduces the original issue.

@MattDevy MattDevy merged commit 844eb0b into main Apr 13, 2026
16 checks passed
@MattDevy MattDevy deleted the MattDevy/fix/issue-99 branch April 13, 2026 17:02
@JoshMock

Copy link
Copy Markdown
Member

Thanks for catching that @MattDevy! ❤️

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.

Connection failures produce empty error message

2 participants