From 90c9802d6d497f58be38a63b1beddd68fbe82707 Mon Sep 17 00:00:00 2001 From: Johannes Fleck Date: Thu, 29 Oct 2026 15:51:11 +0100 Subject: [PATCH 1/3] fix: classify Trino errors before returning them to the client (#304) 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. --- e2e/support/start-mock-trino.ts | 7 +- messages/de.json | 4 + messages/en.json | 4 + .../components/trino/StatementResult.svelte | 19 +++- src/lib/server/trino/client.test.ts | 18 +++- src/lib/server/trino/client.ts | 20 ++--- src/lib/server/trino/errors.test.ts | 56 ++++++++++++ src/lib/server/trino/errors.ts | 88 +++++++++++++++++++ src/lib/server/trino/queries.test.ts | 26 +++--- src/lib/server/trino/queries.ts | 8 +- src/lib/server/trino/result-collector.test.ts | 19 +++- src/lib/server/trino/result-collector.ts | 5 +- src/lib/types/query.ts | 16 +++- src/routes/(app)/api/trino/catalog/+server.ts | 4 +- .../(app)/api/trino/catalog/server.test.ts | 5 +- .../api/trino/completion/metadata/+server.ts | 4 +- .../trino/completion/metadata/server.test.ts | 24 ++++- 17 files changed, 279 insertions(+), 48 deletions(-) create mode 100644 src/lib/server/trino/errors.test.ts create mode 100644 src/lib/server/trino/errors.ts diff --git a/e2e/support/start-mock-trino.ts b/e2e/support/start-mock-trino.ts index 83ebff63..0e26386e 100644 --- a/e2e/support/start-mock-trino.ts +++ b/e2e/support/start-mock-trino.ts @@ -60,7 +60,12 @@ const routes: [string, object][] = [ 'SHOULD_ERROR', { id: 'q-err', - error: { message: 'syntax error at position 7', errorCode: 1 }, + error: { + message: 'syntax error at position 7', + errorCode: 1, + errorName: 'SYNTAX_ERROR', + errorType: 'USER_ERROR' + }, stats: { state: 'FAILED' } } ], diff --git a/messages/de.json b/messages/de.json index bd7844fb..542b24db 100644 --- a/messages/de.json +++ b/messages/de.json @@ -64,6 +64,10 @@ "header_sign_out": "Abmelden", "trino_results_empty": "Keine Ergebnisse", "trino_query_error": "Abfragefehler", + "trino_query_error_server": "Trino konnte die Abfrage wegen eines serverseitigen Fehlers nicht ausführen.", + "trino_query_error_server_named": "Trino konnte die Abfrage wegen eines serverseitigen Fehlers nicht ausführen ({name}).", + "trino_query_error_unavailable": "Trino ist nicht erreichbar oder hat eine ungültige Antwort gesendet.", + "trino_query_error_unknown": "Abfrage fehlgeschlagen", "trino_query_connection_lost": "Verbindung zum Server verloren.", "trino_running": "Wird ausgeführt...", "trino_connection_url_placeholder": "https://trino.example.com:8443", diff --git a/messages/en.json b/messages/en.json index 4a25cd94..58851004 100644 --- a/messages/en.json +++ b/messages/en.json @@ -64,6 +64,10 @@ "header_sign_out": "Sign out", "trino_results_empty": "No results", "trino_query_error": "Query error", + "trino_query_error_server": "Trino could not run the query because of a server-side error.", + "trino_query_error_server_named": "Trino could not run the query because of a server-side error ({name}).", + "trino_query_error_unavailable": "Trino could not be reached or sent an invalid response.", + "trino_query_error_unknown": "Query failed", "trino_query_connection_lost": "Lost connection to the server.", "trino_running": "Running...", "trino_connection_url_placeholder": "https://trino.example.com:8443", diff --git a/src/lib/components/trino/StatementResult.svelte b/src/lib/components/trino/StatementResult.svelte index c49cc466..ff3ee162 100644 --- a/src/lib/components/trino/StatementResult.svelte +++ b/src/lib/components/trino/StatementResult.svelte @@ -5,7 +5,7 @@ import IconExpandMore from 'virtual:icons/material-symbols/expand-more'; import IconOpenInNew from 'virtual:icons/material-symbols/open-in-new'; import IconDownload from 'virtual:icons/material-symbols/download'; - import { MAX_CLIENT_ROWS, type QuerySnapshot } from '$lib/types/query'; + import { MAX_CLIENT_ROWS, type QueryError, type QuerySnapshot } from '$lib/types/query'; import { initPageSize, type PageSize } from '$lib/types/pagination.js'; import Pagination from '$lib/components/Pagination.svelte'; @@ -35,7 +35,7 @@ const lastPage = $derived(totalPages - 1); const rowStart = $derived(currentPage * pageSize + 1); const rowEnd = $derived(currentPage * pageSize + displayedRows.length); - const stmtError = $derived(result.error); + const stmtError = $derived(result.error ? formatQueryError(result.error) : null); const showHeader = $derived(totalStatements > 1); const rowLimitWarning = $derived( showHeader && result.rowLimitReached @@ -43,6 +43,21 @@ : null ); + function formatQueryError(error: QueryError): string { + switch (error.kind) { + case 'query': + return error.message; + case 'server': + return error.name + ? m.trino_query_error_server_named({ name: error.name }) + : m.trino_query_error_server(); + case 'unavailable': + return m.trino_query_error_unavailable(); + case 'unknown': + return m.trino_query_error_unknown(); + } + } + const CSV_HEADERS_KEY = 'trino_csv_include_headers'; let includeHeaders = $state(browser ? localStorage.getItem(CSV_HEADERS_KEY) === 'true' : false); diff --git a/src/lib/server/trino/client.test.ts b/src/lib/server/trino/client.test.ts index a7fd9524..1d9aa202 100644 --- a/src/lib/server/trino/client.test.ts +++ b/src/lib/server/trino/client.test.ts @@ -8,6 +8,7 @@ vi.mock('$env/dynamic/private', () => ({ env: mockEnv })); vi.mock('$lib/server/logging', () => import('$lib/test-utils/mock-logger.js')); import { TrinoClient, trinoMetadataQuery } from './client.js'; +import { TrinoHttpError, TrinoQueryFailure } from './errors.js'; describe('TrinoClient X-Trino-User header', () => { let fetchMock: ReturnType; @@ -151,9 +152,24 @@ describe('trinoMetadataQuery (connection test path)', () => { vi.stubGlobal('fetch', fetchMock); const client = new TrinoClient({ serverUrl: 'http://trino:8080' }); await expect(trinoMetadataQuery(client, 'SELECT 1', { user: 'alice' })).rejects.toThrow( - 'Access Denied' + expect.objectContaining({ + constructor: TrinoQueryFailure, + trinoError: { message: 'Access Denied' } + }) ); }); + + it('throws an HTTP error without the response body when polling fails', async () => { + const fetchMock = vi + .fn() + .mockResolvedValueOnce({ ok: true, json: async () => ({ id: 'q1', nextUri: 'http://n/1' }) }) + .mockResolvedValueOnce({ ok: false, status: 502, text: async () => 'proxy' }); + vi.stubGlobal('fetch', fetchMock); + const client = new TrinoClient({ serverUrl: 'http://trino:8080' }); + const result = trinoMetadataQuery(client, 'SELECT 1', { user: 'alice' }); + await expect(result).rejects.toBeInstanceOf(TrinoHttpError); + await expect(result).rejects.toThrow('Trino poll failed (502)'); + }); }); describe('resolveTrinoPublicUrl', () => { diff --git a/src/lib/server/trino/client.ts b/src/lib/server/trino/client.ts index 0fb801ab..d3ad7aa2 100644 --- a/src/lib/server/trino/client.ts +++ b/src/lib/server/trino/client.ts @@ -2,6 +2,7 @@ import { env } from '$env/dynamic/private'; import { readFileSync } from 'node:fs'; import { Agent } from 'undici'; import { logger } from '$lib/server/logging'; +import { TrinoHttpError, TrinoQueryFailure, type TrinoQueryError } from './errors.js'; import { getUserTrinoClient, getUserTrinoUrl } from './user-clients.js'; const log = logger.child({ module: 'trino-client' }); @@ -20,13 +21,6 @@ export interface TrinoQueryStats { elapsedTimeMillis?: number; } -interface TrinoQueryError { - message: string; - errorCode?: number; - errorName?: string; - errorType?: string; -} - export interface TrinoQueryResult { id: string; nextUri?: string; @@ -91,7 +85,7 @@ export class TrinoClient { if (!res.ok) { const text = await res.text().catch(() => ''); - throw new Error(`Trino POST /v1/statement failed (${res.status}): ${text}`); + throw new TrinoHttpError('POST /v1/statement', res.status, text); } return res.json() as Promise; @@ -112,7 +106,7 @@ export class TrinoClient { if (!res.ok) { const text = await res.text().catch(() => ''); - throw new Error(`Trino poll failed (${res.status}): ${text}`); + throw new TrinoHttpError('poll', res.status, text); } return res.json() as Promise; @@ -139,7 +133,7 @@ export class TrinoClient { // 404 is expected — the query may have already finished and been cleaned up. if (!res.ok && res.status !== 404) { const text = await res.text().catch(() => ''); - throw new Error(`Trino DELETE /v1/query failed (${res.status}): ${text}`); + throw new TrinoHttpError('DELETE /v1/query', res.status, text); } } @@ -154,7 +148,7 @@ export class TrinoClient { if (!res.ok && res.status !== 404 && res.status !== 410) { const text = await res.text().catch(() => ''); - throw new Error(`Trino DELETE nextUri failed (${res.status}): ${text}`); + throw new TrinoHttpError('DELETE nextUri', res.status, text); } } } @@ -267,14 +261,14 @@ export async function trinoMetadataQuery( const rows: unknown[][] = result.data ? [...result.data] : []; if (result.error) { - throw new Error(result.error.message); + throw new TrinoQueryFailure(result.error); } while (result.nextUri) { result = await client.poll(result.nextUri, { user: options.user, signal }); if (result.error) { - throw new Error(result.error.message); + throw new TrinoQueryFailure(result.error); } if (result.columns && columns.length === 0) { columns = result.columns; diff --git a/src/lib/server/trino/errors.test.ts b/src/lib/server/trino/errors.test.ts new file mode 100644 index 00000000..a76661ee --- /dev/null +++ b/src/lib/server/trino/errors.test.ts @@ -0,0 +1,56 @@ +import { describe, it, expect, vi } from 'vitest'; + +vi.mock('$lib/server/logging', () => import('$lib/test-utils/mock-logger.js')); + +import { TrinoHttpError, TrinoQueryFailure, classifyTrinoError, toQueryError } from './errors.js'; + +describe('toQueryError', () => { + it.each(['USER_ERROR', 'INSUFFICIENT_RESOURCES'])('passes on the message of %s', (errorType) => { + expect(toQueryError({ message: 'line 1:8: Column x cannot be resolved', errorType })).toEqual({ + kind: 'query', + message: 'line 1:8: Column x cannot be resolved' + }); + }); + + it.each(['INTERNAL_ERROR', 'EXTERNAL', undefined])( + 'withholds the message of %s and keeps the error name', + (errorType) => { + expect( + toQueryError({ + message: 'Failed connecting to metastore thrift://hive.internal:9083', + errorName: 'HIVE_METASTORE_ERROR', + errorType + }) + ).toEqual({ kind: 'server', name: 'HIVE_METASTORE_ERROR' }); + } + ); + + it.each([undefined, {}, { errorType: 'USER_ERROR' }])( + 'reports a failure without a message as unknown (%o)', + (trinoError) => { + expect(toQueryError(trinoError)).toEqual({ kind: 'unknown' }); + } + ); +}); + +describe('classifyTrinoError', () => { + it('classifies a Trino query failure by its error type', () => { + const err = new TrinoQueryFailure({ message: 'Access Denied', errorType: 'USER_ERROR' }); + expect(classifyTrinoError(err)).toEqual({ kind: 'query', message: 'Access Denied' }); + }); + + it.each([ + ['an HTTP error', new TrinoHttpError('poll', 502, 'proxy.internal')], + ['a network failure', new TypeError('fetch failed')] + ])('reports %s as unavailable', (_, err) => { + expect(classifyTrinoError(err)).toEqual({ kind: 'unavailable' }); + }); +}); + +describe('TrinoHttpError', () => { + it('keeps the response body out of the message and caps it', () => { + const err = new TrinoHttpError('poll', 502, 'x'.repeat(5000)); + expect(err.message).toBe('Trino poll failed (502)'); + expect(err.body).toHaveLength(2000); + }); +}); diff --git a/src/lib/server/trino/errors.ts b/src/lib/server/trino/errors.ts new file mode 100644 index 00000000..7e322d0e --- /dev/null +++ b/src/lib/server/trino/errors.ts @@ -0,0 +1,88 @@ +import { logger } from '$lib/server/logging'; +import type { QueryError } from '$lib/types/query.js'; + +const log = logger.child({ module: 'trino-errors' }); + +/** Error object of a failed Trino query (only fields we read). */ +export interface TrinoQueryError { + message?: string; + errorCode?: number; + errorName?: string; + errorType?: string; +} + +/** Response bodies are kept for logging only; cap them so a proxy's HTML page cannot flood the logs. */ +const MAX_LOGGED_BODY_LENGTH = 2000; + +/** Trino answered with a non-2xx HTTP status. The body never reaches the client. */ +export class TrinoHttpError extends Error { + readonly body: string; + + constructor( + operation: string, + readonly status: number, + body: string + ) { + super(`Trino ${operation} failed (${status})`); + this.name = 'TrinoHttpError'; + this.body = body.slice(0, MAX_LOGGED_BODY_LENGTH); + } +} + +/** Trino reported a failed query. */ +export class TrinoQueryFailure extends Error { + constructor(readonly trinoError: TrinoQueryError) { + super(trinoError.message ?? 'Query failed'); + this.name = 'TrinoQueryFailure'; + } +} + +/** + * Error types whose message describes the user's own query. Trino runs the + * query as the user, so these only mention objects the user may access. + * INTERNAL_ERROR and EXTERNAL messages can contain exception text, hostnames + * or storage paths. + */ +const USER_FACING_ERROR_TYPES = new Set(['USER_ERROR', 'INSUFFICIENT_RESOURCES']); + +/** + * Classify the error object of a failed Trino query. Server errors are logged, + * as their message is withheld from the user. + */ +export function toQueryError( + trinoError: TrinoQueryError | undefined, + trinoQueryId?: string +): QueryError { + if (!trinoError?.message) return { kind: 'unknown' }; + if (USER_FACING_ERROR_TYPES.has(trinoError.errorType ?? '')) { + return { kind: 'query', message: trinoError.message }; + } + log.warn( + { + trino_query_id: trinoQueryId, + trino_error_type: trinoError.errorType, + trino_error_name: trinoError.errorName, + trino_error_message: trinoError.message + }, + 'Trino reported a server error' + ); + return { kind: 'server', name: trinoError.errorName ?? null }; +} + +/** Classify an error thrown while talking to Trino. */ +export function classifyTrinoError(err: unknown): QueryError { + if (err instanceof TrinoQueryFailure) return toQueryError(err.trinoError); + // HTTP errors, network failures and unparsable responses. + return { kind: 'unavailable' }; +} + +/** + * Error body for an API response after a failed Trino call. Only errors in the + * user's query keep their message; others get a generic text with the request ID, + * so the client shows its translated fallback. + */ +export function trinoErrorBody(err: unknown, requestId: string): App.Error { + const queryError = classifyTrinoError(err); + if (queryError.kind === 'query') return { message: queryError.message }; + return { message: 'Trino request failed', requestId }; +} diff --git a/src/lib/server/trino/queries.test.ts b/src/lib/server/trino/queries.test.ts index ebcbbe04..470a45ca 100644 --- a/src/lib/server/trino/queries.test.ts +++ b/src/lib/server/trino/queries.test.ts @@ -1,6 +1,7 @@ import { describe, it, expect, vi, beforeEach, afterEach } from 'vitest'; import type { TrinoClient, TrinoQueryResult } from './client.js'; import type { TrinoQuery } from './queries.js'; +import type { QueryError } from '$lib/types/query.js'; // Stable mock functions shared across module re-imports (each test imports a // fresh queries.ts so its in-memory store starts empty). @@ -48,7 +49,7 @@ function pending(id: string): TrinoQueryResult { } /** Make collectResults drive each polled query to the given terminal state. */ -function collectTo(state: 'FINISHED' | 'FAILED' | 'CANCELLED', error: string | null = null) { +function collectTo(state: 'FINISHED' | 'FAILED' | 'CANCELLED', error: QueryError | null = null) { return async (query: TrinoQuery) => { query.columns = [{ name: 'x', type: 'integer' }]; query.rows = [[1]]; @@ -190,13 +191,13 @@ describe('startScript', () => { }); it('stops at the first statement that does not finish', async () => { - mocks.collectResults.mockImplementationOnce(collectTo('FAILED', 'Table not found')); + mocks.collectResults.mockImplementationOnce(collectTo('FAILED', { kind: 'unknown' })); await q.startScript(trinoClient(), USER, TAB, ['SELECT 1', 'SELECT 2'], OPTS); expect(client.submit).toHaveBeenCalledTimes(1); const [snapshot] = q.getQuerySnapshots(USER, TAB); - expect(snapshot).toMatchObject({ state: 'FAILED', error: 'Table not found' }); + expect(snapshot).toMatchObject({ state: 'FAILED', error: { kind: 'unknown' } }); }); it('marks a statement without nextUri as finished without polling', async () => { @@ -258,7 +259,7 @@ describe('startScript', () => { it('fails and stops on an error in the submit response', async () => { client.submit.mockResolvedValueOnce({ id: 'q1', - error: { message: 'line 1:1: mismatched input' } + error: { message: 'line 1:1: mismatched input', errorType: 'USER_ERROR' } }); await q.startScript(trinoClient(), USER, TAB, ['SELEC 1', 'SELECT 2'], OPTS); // typos:ignore-line @@ -268,7 +269,10 @@ describe('startScript', () => { expect(mocks.activeDec).not.toHaveBeenCalled(); expect(mocks.queryTotalInc).toHaveBeenCalledWith({ outcome: 'failed' }); const [snapshot] = q.getQuerySnapshots(USER, TAB); - expect(snapshot).toMatchObject({ state: 'FAILED', error: 'line 1:1: mismatched input' }); + expect(snapshot).toMatchObject({ + state: 'FAILED', + error: { kind: 'query', message: 'line 1:1: mismatched input' } + }); }); it('treats a FAILED submit response without error details as a failure', async () => { @@ -280,18 +284,10 @@ describe('startScript', () => { expect(mocks.queryTotalInc).not.toHaveBeenCalledWith({ outcome: 'completed' }); expect(q.getQuerySnapshots(USER, TAB)[0]).toMatchObject({ state: 'FAILED', - error: 'Query failed' + error: { kind: 'unknown' } }); }); - it('falls back to a generic message when the submit error has none', async () => { - client.submit.mockResolvedValueOnce({ id: 'q1', error: {} }); - - await q.startScript(trinoClient(), USER, TAB, ['SELECT 1'], OPTS); - - expect(q.getQuerySnapshots(USER, TAB)[0].error).toBe('Query failed'); - }); - it('stops without storing a result when submit throws', async () => { client.submit.mockRejectedValueOnce(new Error('ECONNREFUSED')); @@ -324,7 +320,7 @@ describe('startScript', () => { expect(mocks.queryTotalInc).toHaveBeenCalledWith({ outcome: 'failed' }); expect(q.getQuerySnapshots(USER, TAB)[0]).toMatchObject({ state: 'FAILED', - error: 'Internal poll error' + error: { kind: 'unknown' } }); }); diff --git a/src/lib/server/trino/queries.ts b/src/lib/server/trino/queries.ts index 6161be8c..1ca5f901 100644 --- a/src/lib/server/trino/queries.ts +++ b/src/lib/server/trino/queries.ts @@ -5,11 +5,13 @@ import { INITIAL_PROGRESS, isTerminal, type Column, + type QueryError, type QueryProgress, type QuerySnapshot, type QueryState } from '$lib/types/query.js'; import { type TrinoClient, type TrinoQueryStats, resolveTrinoPublicUrl } from './client.js'; +import { toQueryError } from './errors.js'; import { collectResults } from './result-collector.js'; const log = logger.child({ module: 'trino-queries' }); @@ -28,7 +30,7 @@ export interface TrinoQuery { progress: QueryProgress; columns: Column[]; rows: unknown[][]; - error: string | null; + error: QueryError | null; rowLimitReached: boolean; sql: string; startedAt: number; @@ -218,7 +220,7 @@ async function submitStatement( }; if (submitResult.error || initialState === 'FAILED') { - query.error = submitResult.error?.message ?? 'Query failed'; + query.error = toQueryError(submitResult.error, trinoQueryId); terminateQuery(query, 'FAILED', { decrementGauge: false }); trinoQueryTotal.inc({ outcome: 'failed' }); } @@ -324,7 +326,7 @@ async function runStatements( } catch (err) { log.error({ err, trino_query_id: query.trinoQueryId }, 'poll loop crashed'); if (!isTerminal(query.state)) { - query.error = 'Internal poll error'; + query.error = { kind: 'unknown' }; terminateQuery(query, 'FAILED'); trinoQueryTotal.inc({ outcome: 'failed' }); } diff --git a/src/lib/server/trino/result-collector.test.ts b/src/lib/server/trino/result-collector.test.ts index bdee9d7f..9ab4cceb 100644 --- a/src/lib/server/trino/result-collector.test.ts +++ b/src/lib/server/trino/result-collector.test.ts @@ -10,6 +10,7 @@ vi.mock('$lib/server/metrics.js', () => ({ import { collectResults } from './result-collector.js'; import { MAX_CLIENT_ROWS, INITIAL_PROGRESS } from '$lib/types/query.js'; import type { TrinoQuery } from './queries.js'; +import { TrinoHttpError } from './errors.js'; /** Just over half the limit, so two pages cross MAX_CLIENT_ROWS. */ function page(): unknown[][] { @@ -160,14 +161,26 @@ describe('collectResults failure', () => { const poll = vi.fn(async () => ({ id: 'q1', stats: { state: 'FAILED' }, - error: { message: 'Division by zero' } + error: { message: 'Division by zero', errorType: 'USER_ERROR' } })); const query = makeQuery({ poll }); await collectResults(query); expect(query.state).toBe('FAILED'); - expect(query.error).toBe('Division by zero'); + expect(query.error).toEqual({ kind: 'query', message: 'Division by zero' }); + }); + + it('fails without the response body when polling gets an HTTP error', async () => { + const poll = vi.fn(async () => { + throw new TrinoHttpError('poll', 502, 'internal-proxy.local'); + }); + + const query = makeQuery({ poll }); + await collectResults(query); + + expect(query.state).toBe('FAILED'); + expect(query.error).toEqual({ kind: 'unavailable' }); }); it('fails on a FAILED state without error details', async () => { @@ -185,6 +198,6 @@ describe('collectResults failure', () => { expect(poll).toHaveBeenCalledTimes(1); expect(query.state).toBe('FAILED'); - expect(query.error).toBe('Query failed'); + expect(query.error).toEqual({ kind: 'unknown' }); }); }); diff --git a/src/lib/server/trino/result-collector.ts b/src/lib/server/trino/result-collector.ts index e319d0b0..95419821 100644 --- a/src/lib/server/trino/result-collector.ts +++ b/src/lib/server/trino/result-collector.ts @@ -1,6 +1,7 @@ import { logger } from '$lib/server/logging'; import { trinoQueryTotal } from '$lib/server/metrics.js'; import { MAX_CLIENT_ROWS, isTerminal } from '$lib/types/query.js'; +import { classifyTrinoError, toQueryError } from './errors.js'; import { type TrinoQuery, mapTrinoState, toQueryProgress, terminateQuery } from './queries.js'; const log = logger.child({ module: 'trino-result-collector' }); @@ -30,7 +31,7 @@ export async function collectResults(query: TrinoQuery): Promise { if (isTerminal(query.state)) return; log.error({ err, trino_query_id: query.trinoQueryId }, 'poll failed'); - query.error = err instanceof Error ? err.message : 'Poll failed'; + query.error = classifyTrinoError(err); terminateQuery(query, 'FAILED'); trinoQueryTotal.inc({ outcome: 'failed' }); return; @@ -61,7 +62,7 @@ export async function collectResults(query: TrinoQuery): Promise { } if (result.error || result.stats?.state === 'FAILED') { - query.error = result.error?.message ?? 'Query failed'; + query.error = toQueryError(result.error, query.trinoQueryId); terminateQuery(query, 'FAILED'); trinoQueryTotal.inc({ outcome: 'failed' }); return; diff --git a/src/lib/types/query.ts b/src/lib/types/query.ts index c8a203ab..0ac24176 100644 --- a/src/lib/types/query.ts +++ b/src/lib/types/query.ts @@ -26,6 +26,20 @@ export interface QueryProgress { elapsedTimeMillis: number; } +/** + * Why a query failed, safe to show to the user. Only errors in the user's own + * query carry Trino's message; server-side errors could expose internals. + */ +export type QueryError = + /** Error in the query itself (syntax, missing table, access denied, resource limit). */ + | { kind: 'query'; message: string } + /** Trino or a connector failed; only Trino's error name is passed on. */ + | { kind: 'server'; name: string | null } + /** Trino could not be reached or sent an invalid response. */ + | { kind: 'unavailable' } + /** Failed without details. */ + | { kind: 'unknown' }; + /** Whether the query state is terminal (no further transitions expected). */ export function isTerminal(state: QueryState): boolean { return state === 'FINISHED' || state === 'FAILED' || state === 'CANCELLED'; @@ -40,7 +54,7 @@ export interface QuerySnapshot { rows: unknown[][]; /** Rows fetched so far. Populated even in lightweight snapshots, where `rows` is empty. */ rowCount: number; - error: string | null; + error: QueryError | null; /** Fetching stopped and the query was cancelled because more than MAX_CLIENT_ROWS rows arrived. */ rowLimitReached: boolean; sql: string; diff --git a/src/routes/(app)/api/trino/catalog/+server.ts b/src/routes/(app)/api/trino/catalog/+server.ts index e4ac03b9..59e8a0f5 100644 --- a/src/routes/(app)/api/trino/catalog/+server.ts +++ b/src/routes/(app)/api/trino/catalog/+server.ts @@ -2,6 +2,7 @@ import { json, error } from '@sveltejs/kit'; import { resolveTrinoClient, trinoMetadataQuery } from '$lib/server/trino/client.js'; import { getTrinoUser, getUserId } from '$lib/server/auth-utils.js'; import { quoteIdentifier, quoteStringLiteral } from '$lib/editor/identifiers.js'; +import { trinoErrorBody } from '$lib/server/trino/errors.js'; import type { RequestHandler } from './$types'; export const GET: RequestHandler = async ({ url, locals }) => { @@ -59,8 +60,7 @@ export const GET: RequestHandler = async ({ url, locals }) => { ); return json(rows); } catch (err) { - const msg = err instanceof Error ? err.message : 'unknown error'; log.info({ err, level, catalog, schema, table }, 'catalog metadata query failed'); - error(502, msg); + error(502, trinoErrorBody(err, locals.requestId)); } }; diff --git a/src/routes/(app)/api/trino/catalog/server.test.ts b/src/routes/(app)/api/trino/catalog/server.test.ts index cb6537ac..b8117652 100644 --- a/src/routes/(app)/api/trino/catalog/server.test.ts +++ b/src/routes/(app)/api/trino/catalog/server.test.ts @@ -8,6 +8,7 @@ vi.mock('$lib/server/trino/client.js', () => ({ import { GET } from './+server.js'; import { resolveTrinoClient, trinoMetadataQuery } from '$lib/server/trino/client.js'; import type { TrinoClient } from '$lib/server/trino/client.js'; +import { TrinoQueryFailure } from '$lib/server/trino/errors.js'; const fakeClient = { fake: true } as unknown as TrinoClient; @@ -95,7 +96,9 @@ describe('GET /api/trino/catalog', () => { }); it('throws 502 with the Trino error message when the query fails', async () => { - vi.mocked(trinoMetadataQuery).mockRejectedValue(new Error('Catalog does not exist')); + vi.mocked(trinoMetadataQuery).mockRejectedValue( + new TrinoQueryFailure({ message: 'Catalog does not exist', errorType: 'USER_ERROR' }) + ); await expect(GET(mockEvent({ level: 'schemas', catalog: 'nope' }))).rejects.toThrow( expect.objectContaining({ status: 502, body: { message: 'Catalog does not exist' } }) diff --git a/src/routes/(app)/api/trino/completion/metadata/+server.ts b/src/routes/(app)/api/trino/completion/metadata/+server.ts index 880907be..a25969d8 100644 --- a/src/routes/(app)/api/trino/completion/metadata/+server.ts +++ b/src/routes/(app)/api/trino/completion/metadata/+server.ts @@ -4,6 +4,7 @@ import { resolveTrinoClient, trinoMetadataQuery } from '$lib/server/trino/client import { getTrinoUser, getUserId } from '$lib/server/auth-utils.js'; import { completionEnabled } from '$lib/server/feature-flags.js'; import { quoteIdentifier, quoteStringLiteral } from '$lib/editor/identifiers.js'; +import { trinoErrorBody } from '$lib/server/trino/errors.js'; import type { RequestHandler } from './$types'; // Any name Trino returns is valid; values are quoted/escaped before use in SQL. @@ -115,8 +116,7 @@ export const GET: RequestHandler = async ({ url, locals }) => { ); return json(names); } catch (err) { - const msg = err instanceof Error ? err.message : 'unknown error'; log.info({ err, level, catalog, schema, table }, 'completion metadata query failed'); - error(502, msg); + error(502, trinoErrorBody(err, locals.requestId)); } }; diff --git a/src/routes/(app)/api/trino/completion/metadata/server.test.ts b/src/routes/(app)/api/trino/completion/metadata/server.test.ts index 7e2cca86..3af3db40 100644 --- a/src/routes/(app)/api/trino/completion/metadata/server.test.ts +++ b/src/routes/(app)/api/trino/completion/metadata/server.test.ts @@ -16,6 +16,7 @@ vi.mock('$lib/server/trino/client.js', () => ({ import { GET } from './+server.js'; import { resolveTrinoClient, trinoMetadataQuery } from '$lib/server/trino/client.js'; import type { TrinoClient } from '$lib/server/trino/client.js'; +import { TrinoHttpError, TrinoQueryFailure } from '$lib/server/trino/errors.js'; const fakeClient = { fake: true } as unknown as TrinoClient; @@ -24,7 +25,11 @@ function mockEvent(params: Record) { for (const [k, v] of Object.entries(params)) url.searchParams.set(k, v); return { url, - locals: { logger: { debug: vi.fn(), info: vi.fn() }, user: { id: 'u1', trinoUser: 'alice' } } + locals: { + logger: { debug: vi.fn(), info: vi.fn() }, + requestId: 'req-1', + user: { id: 'u1', trinoUser: 'alice' } + } } as unknown as Parameters[0]; } @@ -154,10 +159,25 @@ describe('GET /api/trino/completion/metadata', () => { }); it('throws 502 with the Trino error message when the query fails', async () => { - vi.mocked(trinoMetadataQuery).mockRejectedValue(new Error('Access Denied')); + vi.mocked(trinoMetadataQuery).mockRejectedValue( + new TrinoQueryFailure({ message: 'Access Denied', errorType: 'USER_ERROR' }) + ); await expect(GET(mockEvent({ level: 'catalogs' }))).rejects.toThrow( expect.objectContaining({ status: 502, body: { message: 'Access Denied' } }) ); }); + + it('throws 502 without upstream details when Trino fails', async () => { + vi.mocked(trinoMetadataQuery).mockRejectedValue( + new TrinoHttpError('POST /v1/statement', 503, 'proxy.internal') + ); + + await expect(GET(mockEvent({ level: 'catalogs' }))).rejects.toThrow( + expect.objectContaining({ + status: 502, + body: { message: 'Trino request failed', requestId: 'req-1' } + }) + ); + }); }); From e36b689f4f6704443d91d411241749b6a085256d Mon Sep 17 00:00:00 2001 From: Johannes Fleck Date: Thu, 29 Oct 2026 15:51:20 +0100 Subject: [PATCH 2/3] test: cover Trino error sanitisation in E2E (#304) 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. --- e2e/support/start-mock-trino.ts | 24 ++++++++++++++++++++++++ e2e/trino/query-lifecycle.spec.ts | 30 +++++++++++++++++++++++++++++- e2e/trino/trino.spec.ts | 1 + 3 files changed, 54 insertions(+), 1 deletion(-) diff --git a/e2e/support/start-mock-trino.ts b/e2e/support/start-mock-trino.ts index 0e26386e..8a89241e 100644 --- a/e2e/support/start-mock-trino.ts +++ b/e2e/support/start-mock-trino.ts @@ -118,6 +118,8 @@ const DEFAULT_RESPONSE = { // FROM exact_limit_table exactly MAX_CLIENT_ROWS rows, then an empty trailing page // FAILS_SILENTLY first response FAILED without an error object // FAILS_LATER one page of rows, then FAILED without an error object +// FAILS_INTERNALLY first response FAILED with an INTERNAL_ERROR +// POLL_UNAVAILABLE QUEUED, then GET answers HTTP 502 with an HTML body // HOLD: RUNNING until released via POST /__mock/release/ // GATE: submit response withheld until released (combinable) // @@ -138,6 +140,8 @@ interface MockQuery { /** Release id for HOLD queries, which stay RUNNING until released. */ hold?: string; cancelled?: boolean; + /** GET nextUri answers HTTP 502, as a proxy in front of an unavailable Trino. */ + pollUnavailable?: boolean; } const queries = new Map(); @@ -254,6 +258,21 @@ function startPagedQuery(id: string, sql: string): Page | null { if (sql.includes('FAILS_SILENTLY')) { return { id, stats: { state: 'FAILED' } }; } + if (sql.includes('FAILS_INTERNALLY')) { + return { + id, + error: { + message: 'java.lang.IllegalStateException: metastore thrift://hive.internal:9083', + errorName: 'GENERIC_INTERNAL_ERROR', + errorType: 'INTERNAL_ERROR' + }, + stats: { state: 'FAILED' } + }; + } + if (sql.includes('POLL_UNAVAILABLE')) { + queries.set(id, { sql, pages: [], pollUnavailable: true }); + return { id, nextUri: nextUri(id, 1), stats: { state: 'QUEUED' } }; + } if (sql.includes('FAILS_LATER')) { queries.set(id, { sql, @@ -338,6 +357,11 @@ http return; } if (req.method === 'GET' && executing) { + if (queries.get(executing[1])?.pollUnavailable) { + res.writeHead(502, { 'Content-Type': 'text/html' }); + res.end('502 Bad Gateway from proxy.internal'); + return; + } pollQuery(executing[1], Number(executing[2])).then((page) => sendJson(res, page)); return; } diff --git a/e2e/trino/query-lifecycle.spec.ts b/e2e/trino/query-lifecycle.spec.ts index 0e9b0b04..3cac4f8d 100644 --- a/e2e/trino/query-lifecycle.spec.ts +++ b/e2e/trino/query-lifecycle.spec.ts @@ -4,7 +4,7 @@ import { mockMarker, mockTrinoStatements, releaseMockTrino } from '../support/mo /** * Query lifecycle against the multi-page mock Trino protocol: polling, running - * state, cancellation and failures reported without error details. + * state, cancellation and failures, including which error details reach the user. */ test.describe('Trino query lifecycle', () => { test.use({ locale: 'en-US' }); @@ -110,6 +110,34 @@ test.describe('Trino query lifecycle', () => { await expect(page.getByText('Query failed')).toBeVisible(); }); + test('withholds the message of a Trino internal error', async ({ page }) => { + await openEditor(page, 'SELECT FAILS_INTERNALLY'); + + await runButton(page).click(); + await waitForQueryComplete(page); + + await expect(statusBadge(page)).toHaveAttribute('data-query-state', 'FAILED'); + await expect( + page.getByText( + 'Trino could not run the query because of a server-side error (GENERIC_INTERNAL_ERROR).' + ) + ).toBeVisible(); + await expect(page.getByText('hive.internal')).toHaveCount(0); + }); + + test('withholds the response body when Trino is unavailable during polling', async ({ page }) => { + await openEditor(page, 'SELECT POLL_UNAVAILABLE'); + + await runButton(page).click(); + await waitForQueryComplete(page); + + await expect(statusBadge(page)).toHaveAttribute('data-query-state', 'FAILED'); + await expect( + page.getByText('Trino could not be reached or sent an invalid response.') + ).toBeVisible(); + await expect(page.getByText('proxy.internal')).toHaveCount(0); + }); + test('re-running between statements stops the previous script', async ({ page }) => { const marker = mockMarker(); const gate = releaseId(); diff --git a/e2e/trino/trino.spec.ts b/e2e/trino/trino.spec.ts index 0f7b2ea1..697b09c4 100644 --- a/e2e/trino/trino.spec.ts +++ b/e2e/trino/trino.spec.ts @@ -133,6 +133,7 @@ test.describe('Trino query editor', () => { // Monaco also renders role="alert" nodes for its own accessibility — filter by content. const alert = page.getByRole('alert').filter({ hasText: 'Query error' }); await expect(alert).toBeVisible(); + // Errors in the user's query keep Trino's message. await expect(alert.getByText('syntax error at position 7')).toBeVisible(); }); From d8eac13b857b535d1c4053cfaf00c07122d3e0b8 Mon Sep 17 00:00:00 2001 From: Johannes Fleck Date: Tue, 3 Nov 2026 12:10:33 +0100 Subject: [PATCH 3/3] fix(trino): Report rejected credentials and failed submits (#304) 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). --- e2e/support/start-mock-trino.ts | 7 ++++ e2e/trino/query-lifecycle.spec.ts | 14 +++++++ messages/de.json | 3 ++ messages/en.json | 3 ++ .../components/trino/StatementResult.svelte | 11 ++++- src/lib/server/trino/errors.test.ts | 7 ++++ src/lib/server/trino/errors.ts | 8 +++- src/lib/server/trino/queries.test.ts | 41 ++++++++++++++++-- src/lib/server/trino/queries.ts | 42 +++++++++++++++++-- src/lib/server/trino/result-collector.ts | 2 +- src/lib/types/query.ts | 4 ++ src/routes/(app)/trino/+page.svelte | 7 +++- 12 files changed, 137 insertions(+), 12 deletions(-) diff --git a/e2e/support/start-mock-trino.ts b/e2e/support/start-mock-trino.ts index 8a89241e..0292ea87 100644 --- a/e2e/support/start-mock-trino.ts +++ b/e2e/support/start-mock-trino.ts @@ -120,6 +120,7 @@ const DEFAULT_RESPONSE = { // FAILS_LATER one page of rows, then FAILED without an error object // FAILS_INTERNALLY first response FAILED with an INTERNAL_ERROR // POLL_UNAVAILABLE QUEUED, then GET answers HTTP 502 with an HTML body +// SUBMIT_UNAUTHORIZED POST answers HTTP 401, as for an expired password // HOLD: RUNNING until released via POST /__mock/release/ // GATE: submit response withheld until released (combinable) // @@ -384,6 +385,12 @@ http user: req.headers['x-trino-user'] as string | undefined }); + if (body.includes('SUBMIT_UNAUTHORIZED')) { + res.writeHead(401, { 'Content-Type': 'text/plain' }); + res.end('Unauthorized: password expired for alice@ldap.internal'); + return; + } + const gate = /GATE:([\w-]+)/.exec(body)?.[1]; if (gate) await waitForRelease(gate); diff --git a/e2e/trino/query-lifecycle.spec.ts b/e2e/trino/query-lifecycle.spec.ts index 3cac4f8d..e2c803d1 100644 --- a/e2e/trino/query-lifecycle.spec.ts +++ b/e2e/trino/query-lifecycle.spec.ts @@ -138,6 +138,20 @@ test.describe('Trino query lifecycle', () => { await expect(page.getByText('proxy.internal')).toHaveCount(0); }); + test('reports rejected credentials when Trino refuses the submit', async ({ page }) => { + await openEditor(page, 'SELECT SUBMIT_UNAUTHORIZED'); + + await runButton(page).click(); + await waitForQueryComplete(page); + + await expect(statusBadge(page)).toHaveAttribute('data-query-state', 'FAILED'); + // .env.test configures Trino via environment, so the deployment's credentials were rejected. + await expect( + page.getByText('Trino rejected the credentials configured for this deployment.') + ).toBeVisible(); + await expect(page.getByText('ldap.internal')).toHaveCount(0); + }); + test('re-running between statements stops the previous script', async ({ page }) => { const marker = mockMarker(); const gate = releaseId(); diff --git a/messages/de.json b/messages/de.json index 542b24db..e4485e5c 100644 --- a/messages/de.json +++ b/messages/de.json @@ -66,6 +66,9 @@ "trino_query_error": "Abfragefehler", "trino_query_error_server": "Trino konnte die Abfrage wegen eines serverseitigen Fehlers nicht ausführen.", "trino_query_error_server_named": "Trino konnte die Abfrage wegen eines serverseitigen Fehlers nicht ausführen ({name}).", + "trino_query_error_unauthenticated_managed": "Trino hat die für diese Installation konfigurierten Anmeldedaten abgelehnt. Wenden Sie sich an Ihre Administration.", + "trino_query_error_unauthenticated_user": "Trino hat die Anmeldedaten Ihrer Verbindung abgelehnt. Prüfen Sie Benutzername und Passwort.", + "trino_query_error_forbidden": "Trino hat die Anfrage abgelehnt. Wenden Sie sich an Ihre Administration, wenn Sie Zugriff benötigen.", "trino_query_error_unavailable": "Trino ist nicht erreichbar oder hat eine ungültige Antwort gesendet.", "trino_query_error_unknown": "Abfrage fehlgeschlagen", "trino_query_connection_lost": "Verbindung zum Server verloren.", diff --git a/messages/en.json b/messages/en.json index 58851004..26f7b8e8 100644 --- a/messages/en.json +++ b/messages/en.json @@ -66,6 +66,9 @@ "trino_query_error": "Query error", "trino_query_error_server": "Trino could not run the query because of a server-side error.", "trino_query_error_server_named": "Trino could not run the query because of a server-side error ({name}).", + "trino_query_error_unauthenticated_managed": "Trino rejected the credentials configured for this deployment. Contact your administrator.", + "trino_query_error_unauthenticated_user": "Trino rejected the credentials in your connection settings. Check the username and password.", + "trino_query_error_forbidden": "Trino refused the request. Contact your administrator if you need access.", "trino_query_error_unavailable": "Trino could not be reached or sent an invalid response.", "trino_query_error_unknown": "Query failed", "trino_query_connection_lost": "Lost connection to the server.", diff --git a/src/lib/components/trino/StatementResult.svelte b/src/lib/components/trino/StatementResult.svelte index ff3ee162..a58d83e8 100644 --- a/src/lib/components/trino/StatementResult.svelte +++ b/src/lib/components/trino/StatementResult.svelte @@ -14,11 +14,14 @@ let { result, index, - totalStatements + totalStatements, + managedConnection }: { result: QuerySnapshot; index: number; totalStatements: number; + /** The connection comes from the deployment, so only an administrator can fix its credentials. */ + managedConnection: boolean; } = $props(); const uid = $props.id(); @@ -51,6 +54,12 @@ return error.name ? m.trino_query_error_server_named({ name: error.name }) : m.trino_query_error_server(); + case 'unauthenticated': + return managedConnection + ? m.trino_query_error_unauthenticated_managed() + : m.trino_query_error_unauthenticated_user(); + case 'forbidden': + return m.trino_query_error_forbidden(); case 'unavailable': return m.trino_query_error_unavailable(); case 'unknown': diff --git a/src/lib/server/trino/errors.test.ts b/src/lib/server/trino/errors.test.ts index a76661ee..7751a480 100644 --- a/src/lib/server/trino/errors.test.ts +++ b/src/lib/server/trino/errors.test.ts @@ -39,6 +39,13 @@ describe('classifyTrinoError', () => { expect(classifyTrinoError(err)).toEqual({ kind: 'query', message: 'Access Denied' }); }); + it.each([ + [401, 'unauthenticated'], + [403, 'forbidden'] + ] as const)('reports HTTP %i as %s', (status, kind) => { + expect(classifyTrinoError(new TrinoHttpError('poll', status, 'denied'))).toEqual({ kind }); + }); + it.each([ ['an HTTP error', new TrinoHttpError('poll', 502, 'proxy.internal')], ['a network failure', new TypeError('fetch failed')] diff --git a/src/lib/server/trino/errors.ts b/src/lib/server/trino/errors.ts index 7e322d0e..1cbcd726 100644 --- a/src/lib/server/trino/errors.ts +++ b/src/lib/server/trino/errors.ts @@ -51,7 +51,7 @@ const USER_FACING_ERROR_TYPES = new Set(['USER_ERROR', 'INSUFFICIENT_RESOURCES'] */ export function toQueryError( trinoError: TrinoQueryError | undefined, - trinoQueryId?: string + trinoQueryId?: string | null ): QueryError { if (!trinoError?.message) return { kind: 'unknown' }; if (USER_FACING_ERROR_TYPES.has(trinoError.errorType ?? '')) { @@ -72,7 +72,11 @@ export function toQueryError( /** Classify an error thrown while talking to Trino. */ export function classifyTrinoError(err: unknown): QueryError { if (err instanceof TrinoQueryFailure) return toQueryError(err.trinoError); - // HTTP errors, network failures and unparsable responses. + if (err instanceof TrinoHttpError) { + if (err.status === 401) return { kind: 'unauthenticated' }; + if (err.status === 403) return { kind: 'forbidden' }; + } + // Other HTTP errors, network failures and unparsable responses. return { kind: 'unavailable' }; } diff --git a/src/lib/server/trino/queries.test.ts b/src/lib/server/trino/queries.test.ts index 470a45ca..70b33497 100644 --- a/src/lib/server/trino/queries.test.ts +++ b/src/lib/server/trino/queries.test.ts @@ -288,7 +288,7 @@ describe('startScript', () => { }); }); - it('stops without storing a result when submit throws', async () => { + it('stops and stores the failed statement when submit throws', async () => { client.submit.mockRejectedValueOnce(new Error('ECONNREFUSED')); await expect( @@ -297,7 +297,27 @@ describe('startScript', () => { expect(client.submit).toHaveBeenCalledTimes(1); expect(mocks.queryTotalInc).toHaveBeenCalledWith({ outcome: 'failed' }); - expect(q.getQuerySnapshots(USER, TAB)).toEqual([]); + expect(q.getQuerySnapshots(USER, TAB)).toEqual([ + expect.objectContaining({ + sql: 'SELECT 1', + state: 'FAILED', + error: { kind: 'unavailable' }, + trinoQueryUrl: null + }) + ]); + }); + + it('reports rejected credentials on submit', async () => { + const { TrinoHttpError } = await import('./errors.js'); + client.submit.mockRejectedValueOnce( + new TrinoHttpError('POST /v1/statement', 401, 'Unauthorized') + ); + + await q.startScript(trinoClient(), USER, TAB, ['SELECT 1'], OPTS); + + expect(q.getQuerySnapshots(USER, TAB)).toEqual([ + expect.objectContaining({ state: 'FAILED', error: { kind: 'unauthenticated' } }) + ]); }); it('stops when Trino returns no query ID', async () => { @@ -307,7 +327,22 @@ describe('startScript', () => { expect(client.submit).toHaveBeenCalledTimes(1); expect(mocks.queryTotalInc).toHaveBeenCalledWith({ outcome: 'failed' }); - expect(q.getQuerySnapshots(USER, TAB)).toEqual([]); + expect(q.getQuerySnapshots(USER, TAB)).toEqual([ + expect.objectContaining({ state: 'FAILED', error: { kind: 'unavailable' } }) + ]); + }); + + it('does not store a failed submit of a superseded script', async () => { + let reject!: (err: Error) => void; + client.submit.mockImplementationOnce(() => new Promise((_, rej) => (reject = rej))); + const first = q.startScript(trinoClient(), USER, TAB, ['SELECT 1'], OPTS); + await vi.waitFor(() => expect(client.submit).toHaveBeenCalledTimes(1)); + + await q.startScript(trinoClient(), USER, TAB, ['SELECT 2'], OPTS); + reject(new Error('ECONNREFUSED')); + await first; + + expect(q.getQuerySnapshots(USER, TAB).map((s) => s.sql)).toEqual(['SELECT 2']); }); it('fails the query when the poll loop crashes', async () => { diff --git a/src/lib/server/trino/queries.ts b/src/lib/server/trino/queries.ts index 67ebe86c..865c8eed 100644 --- a/src/lib/server/trino/queries.ts +++ b/src/lib/server/trino/queries.ts @@ -10,7 +10,7 @@ import { type QueryState } from '$lib/types/query.js'; import { type TrinoClient, type TrinoQueryStats, resolveTrinoPublicUrl } from './client.js'; -import { toQueryError } from './errors.js'; +import { classifyTrinoError, toQueryError } from './errors.js'; import { trinoConfig } from './config.js'; import { collectResults } from './result-collector.js'; @@ -25,7 +25,8 @@ const SWEEP_INTERVAL_MS = 60_000; // --- TrinoQuery model --- export interface TrinoQuery { - trinoQueryId: string; + /** Null if the submit failed before Trino assigned an ID. */ + trinoQueryId: string | null; state: QueryState; progress: QueryProgress; columns: Column[]; @@ -150,7 +151,10 @@ function buildSnapshot( lightweight = false ): QuerySnapshot { return { - trinoQueryUrl: trinoPublicUrl ? `${trinoPublicUrl}/ui/query.html?${query.trinoQueryId}` : null, + trinoQueryUrl: + trinoPublicUrl && query.trinoQueryId + ? `${trinoPublicUrl}/ui/query.html?${query.trinoQueryId}` + : null, state: query.state, progress: query.progress, columns: lightweight ? [] : query.columns, @@ -229,6 +233,33 @@ async function submitStatement( return query; } +/** A statement Trino did not accept, so the user sees why the script stopped. */ +function failedSubmit( + client: TrinoClient, + userId: string, + sql: string, + trinoUser: string, + err: unknown +): TrinoQuery { + const now = Date.now(); + return { + trinoQueryId: null, + state: 'FAILED', + progress: INITIAL_PROGRESS, + columns: [], + rows: [], + error: classifyTrinoError(err), + rowLimitReached: false, + sql, + startedAt: now, + nextUri: undefined, + client, + userId, + trinoUser, + completedAt: now + }; +} + function storeQuery(userId: string, tabId: string, query: TrinoQuery): void { const tabMap = getUserTabMap(userId); const queries = tabMap.get(tabId) ?? []; @@ -241,7 +272,7 @@ async function cancelInTrino(query: TrinoQuery): Promise { try { if (query.nextUri) { await query.client.cancelViaUri(query.nextUri, query.trinoUser); - } else { + } else if (query.trinoQueryId) { await query.client.cancel(query.trinoQueryId, query.trinoUser); } } catch (err) { @@ -300,6 +331,9 @@ async function runStatements( query = await submitStatement(client, userId, tabId, sql, options); } catch (err) { log.error({ err, user_id: userId, tab_id: tabId }, 'failed to submit statement in script'); + // A superseded script must not write into the tab its successor owns. + if (!signal.aborted) + storeQuery(userId, tabId, failedSubmit(client, userId, sql, options.user, err)); break; } diff --git a/src/lib/server/trino/result-collector.ts b/src/lib/server/trino/result-collector.ts index 95419821..c8bd71d8 100644 --- a/src/lib/server/trino/result-collector.ts +++ b/src/lib/server/trino/result-collector.ts @@ -80,7 +80,7 @@ export async function collectResults(query: TrinoQuery): Promise { try { if (result.nextUri) { await query.client.cancelViaUri(result.nextUri, query.trinoUser); - } else { + } else if (query.trinoQueryId) { await query.client.cancel(query.trinoQueryId, query.trinoUser); } } catch (err) { diff --git a/src/lib/types/query.ts b/src/lib/types/query.ts index 0ac24176..ceabcbb9 100644 --- a/src/lib/types/query.ts +++ b/src/lib/types/query.ts @@ -35,6 +35,10 @@ export type QueryError = | { kind: 'query'; message: string } /** Trino or a connector failed; only Trino's error name is passed on. */ | { kind: 'server'; name: string | null } + /** Trino rejected the configured credentials (HTTP 401), e.g. a wrong or expired password. */ + | { kind: 'unauthenticated' } + /** Trino refused the request (HTTP 403), e.g. a denied impersonation. */ + | { kind: 'forbidden' } /** Trino could not be reached or sent an invalid response. */ | { kind: 'unavailable' } /** Failed without details. */ diff --git a/src/routes/(app)/trino/+page.svelte b/src/routes/(app)/trino/+page.svelte index a322350f..e5214181 100644 --- a/src/routes/(app)/trino/+page.svelte +++ b/src/routes/(app)/trino/+page.svelte @@ -777,7 +777,12 @@
{#if runner.results.length > 0} {#each runner.results as result, idx (idx)} - + {/each} {:else if !isActive}

{m.trino_results_empty()}