From 3265ed60db2870250b06645d7f583603e9b4ae70 Mon Sep 17 00:00:00 2001 From: Benedikt Labrenz Date: Fri, 4 Sep 2026 10:48:53 +0200 Subject: [PATCH 1/6] Cancel Trino queries via client-protocol nextUri as the impersonated user --- src/lib/server/trino/client.ts | 28 ++++++- src/lib/server/trino/queries.ts | 8 +- src/lib/server/trino/result-collector.test.ts | 83 +++++++++++++++++++ src/lib/server/trino/result-collector.ts | 7 +- 4 files changed, 122 insertions(+), 4 deletions(-) create mode 100644 src/lib/server/trino/result-collector.test.ts diff --git a/src/lib/server/trino/client.ts b/src/lib/server/trino/client.ts index 8aac3765..8dbd3c85 100644 --- a/src/lib/server/trino/client.ts +++ b/src/lib/server/trino/client.ts @@ -117,11 +117,20 @@ export class TrinoClient { return res.json() as Promise; } + /** commonHeaders plus X-Trino-User for the given user when impersonating. */ + private headersForUser(user: string | undefined): Record { + const headers = { ...this.commonHeaders }; + if (user && (this.impersonate || !this.authenticated)) { + headers['X-Trino-User'] = user; + } + return headers; + } + /** DELETE /v1/query/{queryId} — cancel a running query. */ - async cancel(queryId: string): Promise { + async cancel(queryId: string, user?: string): Promise { const res = await fetch(`${this.serverUrl}/v1/query/${queryId}`, { method: 'DELETE', - headers: this.commonHeaders, + headers: this.headersForUser(user), // @ts-expect-error — Node fetch supports dispatcher via undici dispatcher: this.dispatcher }); @@ -132,6 +141,21 @@ export class TrinoClient { throw new Error(`Trino DELETE /v1/query failed (${res.status}): ${text}`); } } + + /** DELETE nextUri — client-protocol cancel (no kill-query permission needed). */ + async cancelViaUri(uri: string, user?: string): Promise { + const res = await fetch(uri, { + method: 'DELETE', + headers: this.headersForUser(user), + // @ts-expect-error — Node fetch supports dispatcher via undici + dispatcher: this.dispatcher + }); + + 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}`); + } + } } // --- Auth helper --- diff --git a/src/lib/server/trino/queries.ts b/src/lib/server/trino/queries.ts index 0a717406..90d7c7d7 100644 --- a/src/lib/server/trino/queries.ts +++ b/src/lib/server/trino/queries.ts @@ -34,6 +34,7 @@ export interface TrinoQuery { nextUri: string | undefined; client: TrinoClient; userId: string; + trinoUser: string; completedAt: number | null; } @@ -169,6 +170,7 @@ async function submitStatement( nextUri: submitResult.nextUri, client, userId, + trinoUser: options.user, completedAt: null }; @@ -283,7 +285,11 @@ export async function cancelQuery(userId: string, tabId: string): Promise ({ env: {} })); +vi.mock('$lib/server/logging', () => ({ + logger: { child: () => ({ info: vi.fn(), warn: vi.fn(), debug: vi.fn(), error: vi.fn() }) } +})); +vi.mock('$lib/server/metrics.js', () => ({ + trinoQueryTotal: { inc: vi.fn() }, + trinoActiveQueries: { inc: vi.fn(), dec: vi.fn() } +})); + +import { collectResults } from './result-collector.js'; +import { MAX_CLIENT_ROWS, INITIAL_PROGRESS } from '$lib/types/query.js'; +import type { TrinoQuery } from './queries.js'; + +/** Just over half the limit, so two pages cross MAX_CLIENT_ROWS. */ +function page(): unknown[][] { + const count = Math.ceil(MAX_CLIENT_ROWS / 2) + 1; + return Array.from({ length: count }, () => [1]); +} + +function makeQuery(client: Partial): TrinoQuery { + return { + trinoQueryId: 'q1', + state: 'RUNNING', + progress: INITIAL_PROGRESS, + columns: [], + rows: [], + error: null, + sql: 'SELECT * FROM big', + startedAt: 0, + nextUri: 'http://trino/next-1', + client: client as TrinoQuery['client'], + userId: 'u', + trinoUser: 'alice', + completedAt: null + }; +} + +describe('collectResults row limit', () => { + it('cancels via the live nextUri and finishes when MAX_CLIENT_ROWS is exceeded', async () => { + const cancelViaUri = vi.fn(async () => {}); + const cancel = vi.fn(async () => {}); + let call = 0; + const poll = vi.fn(async () => { + call++; + return { + id: 'q1', + data: page(), + nextUri: `http://trino/next-${call + 1}`, + stats: { state: 'RUNNING' } + }; + }); + + const query = makeQuery({ poll, cancelViaUri, cancel }); + await collectResults(query); + + expect(cancelViaUri).toHaveBeenCalledWith('http://trino/next-3', 'alice'); + expect(cancel).not.toHaveBeenCalled(); + expect(query.error).toBe(`ROW_LIMIT:${MAX_CLIENT_ROWS}`); + expect(query.state).toBe('FINISHED'); + expect(query.rows.length).toBeGreaterThanOrEqual(MAX_CLIENT_ROWS); + }); + + it('falls back to cancel(queryId) when the crossing page has no nextUri', async () => { + const cancelViaUri = vi.fn(async () => {}); + const cancel = vi.fn(async () => {}); + const poll = vi.fn(async () => ({ + id: 'q1', + data: Array.from({ length: MAX_CLIENT_ROWS + 1 }, () => [1]), + nextUri: undefined, + stats: { state: 'RUNNING' } + })); + + const query = makeQuery({ poll, cancelViaUri, cancel }); + await collectResults(query); + + expect(cancelViaUri).not.toHaveBeenCalled(); + expect(cancel).toHaveBeenCalledWith('q1', 'alice'); + expect(query.error).toBe(`ROW_LIMIT:${MAX_CLIENT_ROWS}`); + expect(query.state).toBe('FINISHED'); + }); +}); diff --git a/src/lib/server/trino/result-collector.ts b/src/lib/server/trino/result-collector.ts index beb540aa..c9ac2dd9 100644 --- a/src/lib/server/trino/result-collector.ts +++ b/src/lib/server/trino/result-collector.ts @@ -71,7 +71,11 @@ export async function collectResults(query: TrinoQuery): Promise { if (query.rows.length >= MAX_CLIENT_ROWS) { query.error = `ROW_LIMIT:${MAX_CLIENT_ROWS}`; try { - await query.client.cancel(query.trinoQueryId); + if (result.nextUri) { + await query.client.cancelViaUri(result.nextUri, query.trinoUser); + } else { + await query.client.cancel(query.trinoQueryId, query.trinoUser); + } } catch (err) { log.warn({ err, trino_query_id: query.trinoQueryId }, 'failed to cancel after row limit'); } @@ -81,6 +85,7 @@ export async function collectResults(query: TrinoQuery): Promise { } nextUri = result.nextUri; + query.nextUri = nextUri; } // All pages drained. Safe even if cancelled concurrently (terminateQuery From 3c10a7714416ef3f4372c6cfdfd2416e491fd679 Mon Sep 17 00:00:00 2001 From: Benedikt Labrenz Date: Fri, 4 Sep 2026 11:25:24 +0200 Subject: [PATCH 2/6] trino backend improvements --- deploy/helm/cockpit/README.md | 1 + deploy/helm/cockpit/templates/configmap.yaml | 3 + deploy/helm/cockpit/templates/deployment.yaml | 7 + deploy/helm/cockpit/values.yaml | 2 + src/lib/server/trino/client.test.ts | 145 +++++++++++++++++- src/lib/server/trino/client.ts | 33 +++- src/lib/server/trino/queries.ts | 14 +- src/lib/server/trino/result-collector.ts | 2 +- src/lib/server/trino/user-clients.ts | 10 +- src/routes/(app)/trino/+page.server.ts | 51 ++++-- src/routes/(app)/trino/+page.svelte | 8 +- 11 files changed, 241 insertions(+), 35 deletions(-) diff --git a/deploy/helm/cockpit/README.md b/deploy/helm/cockpit/README.md index 3f700eaa..7a34b28b 100644 --- a/deploy/helm/cockpit/README.md +++ b/deploy/helm/cockpit/README.md @@ -105,6 +105,7 @@ Optional pre-configured Trino endpoint. When `trino.url` is set, the in-app conn | Parameter | Description | Default | | --- | --- | --- | | `trino.url` | Trino coordinator URL. | `""` | +| `trino.publicUrl` | Browser-facing URL for "View in Trino" deep links. Defaults to `trino.url`. | `""` | | `trino.userImpersonation.enabled` | Forward the logged-in user to Trino as `X-Trino-User`. When `false`, all queries run as `trino.auth.username` (no per-user authorization/audit in Trino). | `true` | | `trino.userImpersonation.userClaim` | OIDC claim used as the Trino user. Only consumed when impersonation is enabled and OIDC is configured. | `preferred_username` | | `trino.auth.type` | `"none"` or `"basic"`. | `""` | diff --git a/deploy/helm/cockpit/templates/configmap.yaml b/deploy/helm/cockpit/templates/configmap.yaml index 7df6c989..14b74443 100644 --- a/deploy/helm/cockpit/templates/configmap.yaml +++ b/deploy/helm/cockpit/templates/configmap.yaml @@ -16,6 +16,9 @@ data: {{- with .Values.trino.url }} trino-url: {{ . | quote }} {{- end }} + {{- with .Values.trino.publicUrl }} + trino-public-url: {{ . | quote }} + {{- end }} {{- with .Values.trino.auth.type }} trino-auth-type: {{ . | quote }} {{- end }} diff --git a/deploy/helm/cockpit/templates/deployment.yaml b/deploy/helm/cockpit/templates/deployment.yaml index 401e50ee..77a7e370 100644 --- a/deploy/helm/cockpit/templates/deployment.yaml +++ b/deploy/helm/cockpit/templates/deployment.yaml @@ -126,6 +126,13 @@ spec: name: {{ include "cockpit.fullname" . }} key: trino-url {{- end }} + {{- if .Values.trino.publicUrl }} + - name: STACKABLE_COCKPIT_TRINO_PUBLIC_URL + valueFrom: + configMapKeyRef: + name: {{ include "cockpit.fullname" . }} + key: trino-public-url + {{- end }} {{- if .Values.trino.auth.type }} - name: STACKABLE_COCKPIT_TRINO_AUTH_TYPE valueFrom: diff --git a/deploy/helm/cockpit/values.yaml b/deploy/helm/cockpit/values.yaml index 731b061a..6660f8a1 100644 --- a/deploy/helm/cockpit/values.yaml +++ b/deploy/helm/cockpit/values.yaml @@ -161,6 +161,8 @@ auth: trino: # Trino coordinator URL (e.g. https://trino.example.com:8443) url: "" + # Browser-facing URL for "View in Trino" deep links. Defaults to trino.url. + publicUrl: "" userImpersonation: # Forward the logged-in user to Trino as X-Trino-User. When false, all queries # run as trino.auth.username instead (requires basic auth). diff --git a/src/lib/server/trino/client.test.ts b/src/lib/server/trino/client.test.ts index f9bec305..358e2057 100644 --- a/src/lib/server/trino/client.test.ts +++ b/src/lib/server/trino/client.test.ts @@ -1,12 +1,15 @@ -import { describe, it, expect, vi, beforeEach } from 'vitest'; +import { describe, it, expect, vi, beforeEach, afterEach } from 'vitest'; -// Empty env so importing client.ts does not build the env-configured singleton. -vi.mock('$env/dynamic/private', () => ({ env: {} })); +// Mutable mock env — empty by default so importing client.ts does not build the +// env-configured singleton. Tests that need env vars populate it then re-import. +// vi.hoisted ensures the object exists before the hoisted vi.mock factory runs. +const { mockEnv } = vi.hoisted(() => ({ mockEnv: {} as Record })); +vi.mock('$env/dynamic/private', () => ({ env: mockEnv })); vi.mock('$lib/server/logging', () => ({ logger: { child: () => ({ info: vi.fn(), warn: vi.fn(), debug: vi.fn() }) } })); -import { TrinoClient } from './client.js'; +import { TrinoClient, trinoMetadataQuery } from './client.js'; describe('TrinoClient X-Trino-User header', () => { let fetchMock: ReturnType; @@ -42,3 +45,137 @@ describe('TrinoClient X-Trino-User header', () => { expect(submittedHeaders()['X-Trino-User']).toBe('anonymous'); }); }); + +describe('TrinoClient.cancelViaUri', () => { + afterEach(() => vi.unstubAllGlobals()); + + it('sends a DELETE to the given nextUri', async () => { + const fetchMock = vi.fn(async () => ({ ok: true, status: 200, text: async () => '' })); + vi.stubGlobal('fetch', fetchMock); + const client = new TrinoClient({ serverUrl: 'http://trino:8080' }); + const uri = 'http://trino:8080/v1/statement/executing/q1/slug/1'; + await client.cancelViaUri(uri); + expect(fetchMock).toHaveBeenCalledWith(uri, expect.objectContaining({ method: 'DELETE' })); + }); + + it('forwards X-Trino-User when impersonating', async () => { + const fetchMock: ReturnType = vi.fn(async () => ({ + ok: true, + status: 200, + text: async () => '' + })); + vi.stubGlobal('fetch', fetchMock); + const client = new TrinoClient({ serverUrl: 'http://trino:8080', authorization: 'Basic abc' }); + await client.cancelViaUri('http://trino:8080/next', 'alice'); + expect(fetchMock.mock.calls[0][1].headers['X-Trino-User']).toBe('alice'); + }); + + it('omits X-Trino-User when impersonation is disabled', async () => { + const fetchMock: ReturnType = vi.fn(async () => ({ + ok: true, + status: 200, + text: async () => '' + })); + vi.stubGlobal('fetch', fetchMock); + const client = new TrinoClient({ + serverUrl: 'http://trino:8080', + authorization: 'Basic abc', + impersonate: false + }); + await client.cancelViaUri('http://trino:8080/next', 'alice'); + expect(fetchMock.mock.calls[0][1].headers).not.toHaveProperty('X-Trino-User'); + }); + + it('does not throw when the URI is already gone (404/410)', async () => { + for (const status of [404, 410]) { + const fetchMock = vi.fn(async () => ({ ok: false, status, text: async () => '' })); + vi.stubGlobal('fetch', fetchMock); + const client = new TrinoClient({ serverUrl: 'http://trino:8080' }); + await expect(client.cancelViaUri('http://trino:8080/next')).resolves.toBeUndefined(); + } + }); + + it('throws on other non-ok responses', async () => { + const fetchMock = vi.fn(async () => ({ ok: false, status: 500, text: async () => 'boom' })); + vi.stubGlobal('fetch', fetchMock); + const client = new TrinoClient({ serverUrl: 'http://trino:8080' }); + await expect(client.cancelViaUri('http://trino:8080/next')).rejects.toThrow(/500/); + }); +}); + +describe('TrinoClient AbortSignal forwarding', () => { + afterEach(() => vi.unstubAllGlobals()); + + it('forwards the signal to fetch on submit', async () => { + const fetchMock: ReturnType = vi.fn(async () => ({ + ok: true, + json: async () => ({ id: 'q1' }) + })); + vi.stubGlobal('fetch', fetchMock); + const client = new TrinoClient({ serverUrl: 'http://trino:8080' }); + const ac = new AbortController(); + await client.submit('SELECT 1', { user: 'alice', signal: ac.signal }); + expect(fetchMock.mock.calls[0][1].signal).toBe(ac.signal); + }); + + it('forwards the signal to fetch on poll', async () => { + const fetchMock: ReturnType = vi.fn(async () => ({ + ok: true, + json: async () => ({ id: 'q1' }) + })); + vi.stubGlobal('fetch', fetchMock); + const client = new TrinoClient({ serverUrl: 'http://trino:8080' }); + const ac = new AbortController(); + await client.poll('http://trino:8080/next', { signal: ac.signal }); + expect(fetchMock.mock.calls[0][1].signal).toBe(ac.signal); + }); +}); + +describe('trinoMetadataQuery (connection test path)', () => { + afterEach(() => vi.unstubAllGlobals()); + + it('resolves rows for a successful single-page query', async () => { + const fetchMock = vi.fn(async () => ({ + ok: true, + json: async () => ({ id: 'q1', columns: [{ name: '_col0', type: 'integer' }], data: [[1]] }) + })); + vi.stubGlobal('fetch', fetchMock); + const client = new TrinoClient({ serverUrl: 'http://trino:8080' }); + const { rows } = await trinoMetadataQuery(client, 'SELECT 1', { user: 'alice' }); + expect(rows).toEqual([[1]]); + }); + + it('throws when the query returns an error (bad connection/credentials)', async () => { + const fetchMock = vi.fn(async () => ({ + ok: true, + json: async () => ({ id: 'q1', error: { message: 'Access Denied' } }) + })); + vi.stubGlobal('fetch', fetchMock); + const client = new TrinoClient({ serverUrl: 'http://trino:8080' }); + await expect(trinoMetadataQuery(client, 'SELECT 1', { user: 'alice' })).rejects.toThrow( + 'Access Denied' + ); + }); +}); + +describe('resolveTrinoPublicUrl', () => { + afterEach(() => { + for (const key of Object.keys(mockEnv)) delete mockEnv[key]; + vi.resetModules(); + }); + + it('returns the trimmed public URL when configured', async () => { + mockEnv.STACKABLE_COCKPIT_TRINO_URL = 'http://internal:8080'; + mockEnv.STACKABLE_COCKPIT_TRINO_PUBLIC_URL = 'https://public.example.com/'; + vi.resetModules(); + const mod = await import('./client.js'); + expect(mod.resolveTrinoPublicUrl('user1')).toBe('https://public.example.com'); + }); + + it('falls back to the server URL when no public URL is set', async () => { + mockEnv.STACKABLE_COCKPIT_TRINO_URL = 'http://internal:8080'; + vi.resetModules(); + const mod = await import('./client.js'); + expect(mod.resolveTrinoPublicUrl('user1')).toBe('http://internal:8080'); + }); +}); diff --git a/src/lib/server/trino/client.ts b/src/lib/server/trino/client.ts index 8dbd3c85..52f0cdb1 100644 --- a/src/lib/server/trino/client.ts +++ b/src/lib/server/trino/client.ts @@ -71,7 +71,7 @@ export class TrinoClient { /** POST /v1/statement — submit a new query. */ async submit( sql: string, - options: { user: string; catalog?: string; schema?: string } + options: { user: string; catalog?: string; schema?: string; signal?: AbortSignal } ): Promise { const headers: Record = { ...this.commonHeaders, @@ -88,6 +88,7 @@ export class TrinoClient { method: 'POST', headers, body: sql, + signal: options.signal, // @ts-expect-error — Node fetch supports dispatcher via undici dispatcher: this.dispatcher }); @@ -101,10 +102,14 @@ export class TrinoClient { } /** GET nextUri — poll for the next page of results. */ - async poll(nextUri: string): Promise { + async poll( + nextUri: string, + opts: { user?: string; signal?: AbortSignal } = {} + ): Promise { const res = await fetch(nextUri, { method: 'GET', - headers: this.commonHeaders, + headers: this.headersForUser(opts.user), + signal: opts.signal, // @ts-expect-error — Node fetch supports dispatcher via undici dispatcher: this.dispatcher }); @@ -168,6 +173,7 @@ export function buildBasicAuthHeader(username: string, password: string): string // --- Singleton & env config --- const trinoUrl = env.STACKABLE_COCKPIT_TRINO_URL; +const trinoPublicUrl = env.STACKABLE_COCKPIT_TRINO_PUBLIC_URL; const authType = (env.STACKABLE_COCKPIT_TRINO_AUTH_TYPE ?? 'none') as 'none' | 'basic'; const authUsername = env.STACKABLE_COCKPIT_TRINO_AUTH_USERNAME; const authPassword = env.STACKABLE_COCKPIT_TRINO_AUTH_PASSWORD; @@ -213,6 +219,7 @@ if (trinoConfigured) { log.info( { trino_url: trinoUrl, + trino_public_url: trinoPublicUrl, auth_type: authType, tls_insecure: tlsInsecure, user_impersonation: trinoUserImpersonation @@ -230,24 +237,34 @@ export function resolveTrinoClient(userId: string): TrinoClient | null { return getUserTrinoClient(userId); } -/** Returns the Trino server URL for a user (for building UI links). */ +/** Returns the Trino server URL used for server-side queries. */ export function resolveTrinoServerUrl(userId: string): string | null { if (trinoUrl) return trinoUrl.replace(/\/+$/, ''); return getUserTrinoUrl(userId); } +/** Public (browser-facing) Trino URL for UI deep links; falls back to the query URL. */ +export function resolveTrinoPublicUrl(userId: string): string | null { + if (trinoPublicUrl) return trinoPublicUrl.replace(/\/+$/, ''); + return resolveTrinoServerUrl(userId); +} + // --- Metadata helper --- +/** Save-time connection-test timeout. */ +export const CONN_TEST_TIMEOUT_MS = 10_000; + /** * Submit a SQL query and drain all pages, returning columns and rows. - * Used for simple metadata queries (catalog browser). + * An optional AbortSignal bounds the whole submit/poll run. */ export async function trinoMetadataQuery( client: TrinoClient, sql: string, - options: { user: string; catalog?: string; schema?: string } + options: { user: string; catalog?: string; schema?: string }, + signal?: AbortSignal ): Promise<{ columns: TrinoColumn[]; rows: unknown[][] }> { - let result = await client.submit(sql, options); + let result = await client.submit(sql, { ...options, signal }); let columns: TrinoColumn[] = result.columns ?? []; const rows: unknown[][] = result.data ? [...result.data] : []; @@ -257,7 +274,7 @@ export async function trinoMetadataQuery( } while (result.nextUri) { - result = await client.poll(result.nextUri); + result = await client.poll(result.nextUri, { user: options.user, signal }); if (result.error) { throw new Error(result.error.message); diff --git a/src/lib/server/trino/queries.ts b/src/lib/server/trino/queries.ts index 90d7c7d7..935bebcc 100644 --- a/src/lib/server/trino/queries.ts +++ b/src/lib/server/trino/queries.ts @@ -9,7 +9,7 @@ import { type QuerySnapshot, type QueryState } from '$lib/types/query.js'; -import { type TrinoClient, type TrinoQueryStats, resolveTrinoServerUrl } from './client.js'; +import { type TrinoClient, type TrinoQueryStats, resolveTrinoPublicUrl } from './client.js'; import { collectResults } from './result-collector.js'; const log = logger.child({ module: 'trino-queries' }); @@ -111,11 +111,11 @@ function getActiveQuery(userId: string, tabId: string): TrinoQuery | undefined { function buildSnapshot( query: TrinoQuery, - trinoServerUrl: string | null, + trinoPublicUrl: string | null, lightweight = false ): QuerySnapshot { return { - trinoQueryUrl: trinoServerUrl ? `${trinoServerUrl}/ui/query.html?${query.trinoQueryId}` : null, + trinoQueryUrl: trinoPublicUrl ? `${trinoPublicUrl}/ui/query.html?${query.trinoQueryId}` : null, state: query.state, progress: query.progress, columns: lightweight ? [] : query.columns, @@ -246,18 +246,18 @@ export function getQuerySnapshots( lightweight = false ): QuerySnapshot[] { touchTab(userId, tabId); - const trinoServerUrl = resolveTrinoServerUrl(userId); - return getTabQueries(userId, tabId).map((q) => buildSnapshot(q, trinoServerUrl, lightweight)); + const trinoPublicUrl = resolveTrinoPublicUrl(userId); + return getTabQueries(userId, tabId).map((q) => buildSnapshot(q, trinoPublicUrl, lightweight)); } /** Lightweight summaries without rows/columns — used for SSR to keep the payload small. */ export function getAllQuerySummaries(userId: string): Record { const tabMap = userQueries.get(userId); if (!tabMap) return {}; - const trinoServerUrl = resolveTrinoServerUrl(userId); + const trinoPublicUrl = resolveTrinoPublicUrl(userId); const result: Record = {}; for (const [tabId, queries] of tabMap) { - result[tabId] = queries.map((q) => buildSnapshot(q, trinoServerUrl, true)); + result[tabId] = queries.map((q) => buildSnapshot(q, trinoPublicUrl, true)); } return result; } diff --git a/src/lib/server/trino/result-collector.ts b/src/lib/server/trino/result-collector.ts index c9ac2dd9..841b7cb4 100644 --- a/src/lib/server/trino/result-collector.ts +++ b/src/lib/server/trino/result-collector.ts @@ -25,7 +25,7 @@ export async function collectResults(query: TrinoQuery): Promise { let result; try { - result = await query.client.poll(nextUri); + result = await query.client.poll(nextUri, { user: query.trinoUser }); } catch (err) { if (isTerminal(query.state)) return; diff --git a/src/lib/server/trino/user-clients.ts b/src/lib/server/trino/user-clients.ts index 24a75879..90a538f5 100644 --- a/src/lib/server/trino/user-clients.ts +++ b/src/lib/server/trino/user-clients.ts @@ -17,19 +17,23 @@ interface UserEntry { const userClients = new Map(); -/** Create or replace the per-user Trino connection. */ -export function createUserTrinoClient(userId: string, config: UserConnectionConfig): void { +/** Build a per-user Trino client from a connection config without storing it. */ +export function buildUserTrinoClient(config: UserConnectionConfig): TrinoClient { const authorization = config.authType === 'basic' && config.username && config.password ? buildBasicAuthHeader(config.username, config.password) : undefined; - const client = new TrinoClient({ + return new TrinoClient({ serverUrl: config.url, authorization, impersonate: trinoUserImpersonation }); +} +/** Create or replace the per-user Trino connection. */ +export function createUserTrinoClient(userId: string, config: UserConnectionConfig): void { + const client = buildUserTrinoClient(config); userClients.set(userId, { client, url: config.url }); log.info({ user_id: userId, trino_url: config.url }, 'user connection created'); } diff --git a/src/routes/(app)/trino/+page.server.ts b/src/routes/(app)/trino/+page.server.ts index 38dcb714..e3653a95 100644 --- a/src/routes/(app)/trino/+page.server.ts +++ b/src/routes/(app)/trino/+page.server.ts @@ -3,9 +3,17 @@ import { superValidate, message } from 'sveltekit-superforms'; import { zod4 as zod } from 'sveltekit-superforms/adapters'; import { getUserId } from '$lib/server/auth-utils.js'; import { getAllQuerySummaries, cancelQuery } from '$lib/server/trino/queries.js'; -import { trinoConfigured } from '$lib/server/trino/client.js'; +import { + trinoConfigured, + trinoMetadataQuery, + CONN_TEST_TIMEOUT_MS +} from '$lib/server/trino/client.js'; import { completionEnabled } from '$lib/server/feature-flags.js'; -import { createUserTrinoClient, getUserTrinoClient } from '$lib/server/trino/user-clients.js'; +import { + buildUserTrinoClient, + createUserTrinoClient, + getUserTrinoClient +} from '$lib/server/trino/user-clients.js'; import { ConnectionSchema, type ConnectionMessage } from './validation.js'; import type { Actions, PageServerLoad } from './$types'; @@ -34,20 +42,43 @@ export const actions: Actions = { } const userId = getUserId(locals); + const user = locals.user?.username ?? 'anonymous'; + const { connectionUrl, authType, authUsername, authPassword } = form.data; + const config = { + url: connectionUrl, + authType, + username: authUsername, + password: authPassword + }; + + // Verify connectivity before storing the client or cancelling running queries. + try { + const testClient = buildUserTrinoClient(config); + await trinoMetadataQuery( + testClient, + 'SELECT 1', + { user }, + AbortSignal.timeout(CONN_TEST_TIMEOUT_MS) + ); + } catch (err) { + const name = (err as { name?: string })?.name; + const detail = (err as { message?: string })?.message ?? 'unknown error'; + const reason = + name === 'TimeoutError' || name === 'AbortError' + ? 'Connection test timed out' + : `Could not connect to Trino: ${detail}`; + log.info({ err, trino_url: connectionUrl }, 'connection test failed'); + return message(form, { type: 'error', message: reason } satisfies ConnectionMessage, { + status: 400 + }); + } // Cancel any running query before replacing the connection. for (const tabId of Object.keys(getAllQuerySummaries(userId))) { await cancelQuery(userId, tabId); } - const { connectionUrl, authType, authUsername, authPassword } = form.data; - - createUserTrinoClient(userId, { - url: connectionUrl, - authType, - username: authUsername, - password: authPassword - }); + createUserTrinoClient(userId, config); log.info({ trino_url: connectionUrl }, 'user connection saved'); return message(form, { type: 'success' } satisfies ConnectionMessage); diff --git a/src/routes/(app)/trino/+page.svelte b/src/routes/(app)/trino/+page.svelte index 97db2496..fe56f01e 100644 --- a/src/routes/(app)/trino/+page.svelte +++ b/src/routes/(app)/trino/+page.svelte @@ -135,7 +135,8 @@ const { enhance: connectionEnhance, errors: connectionErrors, - message: connectionMessage + message: connectionMessage, + submitting: connectionSubmitting } = superForm( untrack(() => data.connectionForm), { @@ -581,7 +582,10 @@
-
From 773829aa69e2543d8c015129f90bf2c10c034839 Mon Sep 17 00:00:00 2001 From: Benedikt Labrenz Date: Fri, 4 Sep 2026 12:06:32 +0200 Subject: [PATCH 3/6] Make the catalog browser scrollable and resizable --- e2e/trino/catalog-browser.spec.ts | 57 +++++++++++++ messages/de.json | 1 + messages/en.json | 1 + src/lib/components/catalog/CatalogTree.svelte | 4 +- .../storage/sidebar/ResizeHandle.svelte | 6 +- src/routes/(app)/trino/+page.svelte | 82 +++++++++++-------- 6 files changed, 115 insertions(+), 36 deletions(-) diff --git a/e2e/trino/catalog-browser.spec.ts b/e2e/trino/catalog-browser.spec.ts index be0180ed..be0e0d7e 100644 --- a/e2e/trino/catalog-browser.spec.ts +++ b/e2e/trino/catalog-browser.spec.ts @@ -129,6 +129,63 @@ test.describe('Catalog browser', () => { await schemaSelect.selectOption('sf1'); }); + test('long schema lists scroll instead of being clipped', async ({ page }) => { + // Inject many schemas so the tree must scroll within its bounded panel height. + await page.route( + (url) => + url.pathname.endsWith('/api/trino/catalog') && url.searchParams.get('level') === 'schemas', + async (route) => { + const schemas = Array.from({ length: 60 }, (_, i) => [ + `schema_${String(i).padStart(2, '0')}` + ]); + await route.fulfill({ json: schemas }); + } + ); + + await ensureCatalogBrowserOpen(page); + const browser = page.getByRole('navigation', { name: 'Catalog browser' }); + + await browser.getByRole('button', { name: 'tpch' }).click(); + await expect(browser.getByText('schema_00', { exact: true })).toBeVisible(); + await expect(browser.getByText('schema_59', { exact: true })).toBeAttached(); + + // The tree container must have a bounded height and actually scroll. + const scrollable = browser.locator('div.overflow-auto').first(); + const canScroll = await scrollable.evaluate((el) => el.scrollHeight > el.clientHeight + 1); + expect(canScroll).toBe(true); + + const scrolled = await scrollable.evaluate((el) => { + el.scrollTop = el.scrollHeight; + return el.scrollTop > 0; + }); + expect(scrolled).toBe(true); + }); + + test('catalog browser width is resizable and persists across reload', async ({ page }) => { + await ensureCatalogBrowserOpen(page); + + const handle = page.getByRole('separator', { name: 'Resize catalog browser' }); + await expect(handle).toBeVisible(); + + const before = Number(await handle.getAttribute('aria-valuenow')); + await handle.focus(); + await page.keyboard.press('Shift+ArrowRight'); // +20px + + await expect(handle).toHaveAttribute('aria-valuenow', String(before + 20)); + + const stored = await page.evaluate(() => localStorage.getItem('trino_catalog_browser_width')); + expect(Number(stored)).toBe(before + 20); + + // Width persists across reload. + await page.reload(); + await waitForHydration(page); + await ensureCatalogBrowserOpen(page); + await expect(page.getByRole('separator', { name: 'Resize catalog browser' })).toHaveAttribute( + 'aria-valuenow', + String(before + 20) + ); + }); + test('browser is hidden by default on mobile', async ({ page }) => { // Set mobile viewport. await page.setViewportSize({ width: 375, height: 667 }); diff --git a/messages/de.json b/messages/de.json index c54247fe..fee6950e 100644 --- a/messages/de.json +++ b/messages/de.json @@ -89,6 +89,7 @@ "trino_table_type_view": "View", "trino_table_type_materialized_view": "Mat. View", "trino_catalog_refresh": "Katalog aktualisieren", + "trino_catalog_resize_handle": "Katalogbrowser-Breite anpassen", "trino_catalog_error": "Katalog konnte nicht geladen werden", "trino_catalog_load_children_error": "Laden fehlgeschlagen", "trino_view_in_trino": "In Trino anzeigen", diff --git a/messages/en.json b/messages/en.json index 55907765..c035e359 100644 --- a/messages/en.json +++ b/messages/en.json @@ -89,6 +89,7 @@ "trino_table_type_view": "View", "trino_table_type_materialized_view": "Mat. view", "trino_catalog_refresh": "Refresh catalog", + "trino_catalog_resize_handle": "Resize catalog browser", "trino_catalog_error": "Failed to load catalog", "trino_catalog_load_children_error": "Failed to load", "trino_view_in_trino": "View in Trino", diff --git a/src/lib/components/catalog/CatalogTree.svelte b/src/lib/components/catalog/CatalogTree.svelte index b9640542..68f05977 100644 --- a/src/lib/components/catalog/CatalogTree.svelte +++ b/src/lib/components/catalog/CatalogTree.svelte @@ -192,7 +192,7 @@ {#if node.type === 'table' || node.type === 'view' || node.type === 'materialized_view'}