Repository navigation
Sanitise trino errors - #430
Merged
Merged
Conversation
Trino error messages and raw upstream response bodies were passed to the browser verbatim. Errors are now classified on the server: - USER_ERROR and INSUFFICIENT_RESOURCES keep Trino's message, as they describe the user's own query. - INTERNAL_ERROR, EXTERNAL and unknown types only expose the error name; the message is logged. - HTTP and network failures are reported as unavailable; the response body is only logged. Query snapshots carry the classification as a structured error that the client translates. The catalog and completion metadata endpoints return a generic message with the request ID for anything but query errors.
Add mock Trino markers for an internal error and an HTTP 502 while polling, and check that the UI shows the generic text without the internal details.
Coverage Report
File Coverage
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
Bohreromir
marked this pull request as ready for review
October 29, 2026 15:34
adwk67
self-requested a review
November 2, 2026 14:37
adwk67
moved this from Development: Waiting for Review
to Development: In Review
in Stackable Engineering
adwk67
previously approved these changes
Nov 2, 2026
adwk67
left a comment
adwk67
left a comment
Member
There was a problem hiding this comment.
Nit: if the config contains a wrong password, the server logs a 401 internally
[16:39:29.394] ERROR: failed to submit statement in script
service: "stackable-cockpit"
module: "trino-queries"
user_id: "oYdJDHf0VhTXnqHN8tzT5cVkT03NuYgV"
tab_id: "d3c19c72-0a47-4276-8585-ef529b9a3859"
err: {
"type": "TrinoHttpError",
"message": "Trino POST /v1/statement failed (401)",
"stack":
TrinoHttpError: Trino POST /v1/statement failed (401)
at TrinoClient.submit (/home/andrew/gitrepos/cockpit/src/lib/server/trino/client.ts:60:13)
at process.processTicksAndRejections (node:internal/process/task_queues:104:5)
at async submitStatement (/home/andrew/gitrepos/cockpit/src/lib/server/trino/queries.ts:126:20)
at async runStatements (/home/andrew/gitrepos/cockpit/src/lib/server/trino/queries.ts:201:15)
at async startScript (/home/andrew/gitrepos/cockpit/src/lib/server/trino/queries.ts:191:5)
"status": 401,
"body": "Access Denied: Invalid credentials",
"name": "TrinoHttpError"
}
but then never gets out of a loop on
[16:39:29.613] TRACE: Request started
service: "stackable-cockpit"
request_id: "81043d6e-5a62-4b2f-80d7-7a787f61083d"
method: "GET"
path: "/api/trino/query"
[16:39:29.615] TRACE: query poll
service: "stackable-cockpit"
request_id: "81043d6e-5a62-4b2f-80d7-7a787f61083d"
method: "GET"
path: "/api/trino/query"
user_id: "oYdJDHf0VhTXnqHN8tzT5cVkT03NuYgV"
tab_id: "d3c19c72-0a47-4276-8585-ef529b9a3859"
count: 0
[16:39:29.615] TRACE: Request completed
service: "stackable-cockpit"
request_id: "81043d6e-5a62-4b2f-80d7-7a787f61083d"
method: "GET"
path: "/api/trino/query"
status_code: 200
duration_ms: 1
and the editor is stuck on "Running...". This might also happen if Trino happens to be unavailable the moment someone clicks on "Run" (though I haven't tested that).
Can we improve on the error feedback in this case? (According to Claude this is the same on main, so not strictly something to do with this PR).
…tise-trino-errors # Conflicts: # src/lib/server/trino/client.ts # src/lib/server/trino/queries.ts
A submit that Trino rejected stored nothing for the tab, so the browser kept polling an empty list and the editor stayed on "Running...". The failed statement is now stored as FAILED with the classified error. HTTP 401 and 403 from Trino get their own error kinds instead of being reported as unavailable. For 401 the message depends on where the credentials come from: the deployment configuration (contact an administrator) or the user's own connection (check the settings).
Bohreromir
moved this from Development: In Review
to Development: Done
in Stackable Engineering
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.
Description
This classifies Trino errors and passes some to the user and keeping potentially sensitive ones to the server while informing the user generic
closes #304
Docs
Release note
Checklist
Author
Reviewer
Acceptance