feat: classify connection failures into actionable tool errors (#266)#339
Merged
Conversation
Co-Authored-By: Claude Opus 4.8 (1M context) <[email protected]>
) Post-review hardening from the final code review: - tryClassifyConnectionError wraps getSourceConfig so it never throws from within a caller's catch block (matches classifyConnectionError's totality). - Add errno 1698 (ER_ACCESS_DENIED_NO_PASSWORD_ERROR) to the mysql/mariadb auth table — the most likely auth failure that previously slipped through to EXECUTION_ERROR. Co-Authored-By: Claude Opus 4.8 (1M context) <[email protected]>
Contributor
There was a problem hiding this comment.
Pull request overview
This PR improves DBHub tool error observability by classifying common connection/access failures into actionable, machine-readable tool error codes (SOURCE_UNREACHABLE, AUTH_FAILED, TUNNEL_FAILED) with templated, source-aware messages, while leaving all unrecognized errors on the existing generic paths.
Changes:
- Added a pure
classifyConnectionError()utility and atryClassifyConnectionError()helper to map low-level connection/auth/tunnel failures into actionable tool errors. - Marked SSH tunnel establishment failures in
ConnectorManagerso they can be classified asTUNNEL_FAILED. - Wired classification into
execute_sql,search_objects, and custom tool execution paths, with new unit tests covering the expected behaviors.
Reviewed changes
Copilot reviewed 10 out of 10 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| src/utils/tool-handler-helpers.ts | Adds tryClassifyConnectionError() to centralize source config lookup + classification + tool error formatting. |
| src/utils/error-classifier.ts | Introduces error classification logic and error code/message generation. |
| src/utils/tests/error-classifier.test.ts | Unit tests for the classifier’s network/auth/tunnel/null behaviors. |
| src/connectors/manager.ts | Tags SSH tunnel establishment errors with a marker for TUNNEL_FAILED classification. |
| src/tools/execute-sql.ts | Uses classification in the catch path before generic EXECUTION_ERROR. |
| src/tools/search-objects.ts | Uses classification in the catch path before generic SEARCH_ERROR. |
| src/tools/custom-tool-handler.ts | Classifies connection/access failures before augmenting errors with SQL/debug context. |
| src/tools/tests/execute-sql.test.ts | Adds tests for SOURCE_UNREACHABLE, fallback behavior, and single-source "default" display id. |
| src/tools/tests/search-objects.test.ts | Adds a test asserting AUTH_FAILED mapping for a connector login error. |
| src/tools/tests/custom-tool-handler.test.ts | Adds a test ensuring connection failures return SOURCE_UNREACHABLE and are not SQL-augmented. |
) Co-Authored-By: Claude Opus 4.8 (1M context) <[email protected]>
…#266) The message hard-coded 'connection refused or timed out' but the classifier also matches ENOTFOUND/EHOSTUNREACH/ENETUNREACH/ECONNRESET. Drop the over-specific parenthetical; the remediation already names host/port/network. Co-Authored-By: Claude Opus 4.8 (1M context) <[email protected]>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Addresses the observability ask in #266 (bkurinsky's StrongDM/ephemeral-access case) — not by adding a status/probe tool, but by making the error a real query already produces actionable.
Today every failure — bad SQL, connection refused, auth expired, dead SSH tunnel — collapses into one generic
EXECUTION_ERRORwith the raw driver message, so an agent can't tell "fix your SQL" from "the source is down / refresh access". This classifies connection/access failures into three distinct codes:SOURCE_UNREACHABLE— socket failures (ECONNREFUSED,ETIMEDOUT,ENOTFOUND,EHOSTUNREACH,ENETUNREACH,ECONNRESET)AUTH_FAILED— per-driver auth signals (pg28P01/28000; mysql/mariadbER_ACCESS_DENIED_ERROR/1045/1698; sqlserverELOGIN)TUNNEL_FAILED— SSH tunnel establishment failuresEach returns a templated, source-named, actionable message (e.g. "Source 'staging' is unreachable (connection refused or timed out). Verify the database is running and reachable…") plus
details: { source_id }. Anything unrecognized falls through to the existingEXECUTION_ERROR/SEARCH_ERRORpath unchanged — no regression.How it works
src/utils/error-classifier.ts— pureclassifyConnectionError(error, connectorType, sourceId), detects onerror.code/errnoonly (never message text); returnsnullfor anything it doesn't recognize.src/connectors/manager.ts—connectSourcetags SSH-tunnel-establishment failures with a marker so they classify asTUNNEL_FAILEDrather than a plain network error.src/utils/tool-handler-helpers.ts—tryClassifyConnectionErrorhelper centralizes the raw-source-id (config lookup) vs display-source-id (message) contract; resolves the connector type from the source config so it works even when the connect itself failed before a connector instance exists (the lazy/revoked-source case).execute_sql,search_objects, and custom tools (the custom handler classifies before appending itsSQL: …debug context, since a down source isn't a SQL problem).Scope (intentionally minimal, per design doc)
No new MCP tool, no probe/status tool, no
list_sources, no live reachability checking, no retry logic, no SQLite-specific handling. The companion context-reduction ask from #266 was handled separately in #338.Test plan
pnpm test src/tools/__tests__ src/utils/__tests__ src/config— 702 unit tests passpnpm run build— cleanSOURCE_UNREACHABLE, auth (code + errno) →AUTH_FAILED, tunnel marker precedence →TUNNEL_FAILED, unrecognized →null/generic fallback, single-sourcesource_id: "default"contract, custom-tool classify-before-SQL-suffix🤖 Generated with Claude Code