diff --git a/e2e/auth/session-expiry.spec.ts b/e2e/auth/session-expiry.spec.ts index 745d3da2..c0d5635d 100644 --- a/e2e/auth/session-expiry.spec.ts +++ b/e2e/auth/session-expiry.spec.ts @@ -93,11 +93,22 @@ test.describe('Session expiry', () => { await page.goto('/'); await waitForHydration(page); - // The storage API answers 401 when no storage connection header is sent - // (requires the storage browser to be enabled, as in .env.test). - const status = await fetchAndAwaitSessionCheck(page, '/api/storage/buckets'); + await page.route('**/api/trino/catalog?*', (route) => + route.fulfill({ status: 401, json: { message: 'Authentication required' } }) + ); + const status = await fetchAndAwaitSessionCheck(page, '/api/trino/catalog?level=catalogs'); expect(status).toBe(401); await expect(page.getByRole('dialog', { name: 'Session expired' })).toBeHidden(); }); + + test('a storage call without a connection is not mistaken for an expired session', async ({ + page + }) => { + // Requires the storage browser to be enabled, as in .env.test. + const res = await page.request.get('/api/storage/buckets'); + + expect(res.status()).toBe(409); + expect(await res.json()).toMatchObject({ reason: 'not-connected' }); + }); }); diff --git a/messages/de.json b/messages/de.json index 232ef271..9e35dd45 100644 --- a/messages/de.json +++ b/messages/de.json @@ -648,11 +648,12 @@ "storage_details_unavailable": "Konnte nicht geladen werden", "storage_details_custom_metadata": "Benutzerdefinierte Metadaten", "storage_details_error_not_connected": "Keine Speicherverbindung konfiguriert", - "storage_details_error_fetch_file": "Dateidetails konnten nicht abgerufen werden ({status})", - "storage_details_error_fetch_bucket": "Bucket-Details konnten nicht abgerufen werden ({status})", - "storage_details_error_fetch_dir_meta": "Verzeichnismetadaten konnten nicht abgerufen werden ({status})", + "storage_details_error_fetch_file": "Die Dateidetails konnten nicht geladen werden.", + "storage_details_error_fetch_bucket": "Die Bucket-Details konnten nicht geladen werden.", + "storage_details_error_fetch_dir_meta": "Die Verzeichnismetadaten konnten nicht geladen werden.", + "storage_details_error_access_denied": "Sie haben keine Berechtigung, diese Details anzuzeigen.", + "storage_details_error_not_found": "Nicht gefunden. Möglicherweise wurde es verschoben oder gelöscht.", "storage_details_error_calc_size": "Die Verzeichnisgröße konnte nicht berechnet werden. Bitte versuchen Sie es erneut.", - "storage_details_error_unknown": "Unbekannter Fehler", "storage_details_error_no_body": "Kein Antworttext", "storage_details_unnamed_rule": "(unbenannt)", "storage_details_treemap_aria": "Treemap-Visualisierung der Verzeichnisgrößenverteilung", diff --git a/messages/en.json b/messages/en.json index 00d6bc5d..72d650f6 100644 --- a/messages/en.json +++ b/messages/en.json @@ -648,11 +648,12 @@ "storage_details_unavailable": "Could not load", "storage_details_custom_metadata": "Custom Metadata", "storage_details_error_not_connected": "No storage connection configured", - "storage_details_error_fetch_file": "Failed to fetch file details ({status})", - "storage_details_error_fetch_bucket": "Failed to fetch bucket details ({status})", - "storage_details_error_fetch_dir_meta": "Failed to fetch directory metadata ({status})", + "storage_details_error_fetch_file": "Failed to load the file details.", + "storage_details_error_fetch_bucket": "Failed to load the bucket details.", + "storage_details_error_fetch_dir_meta": "Failed to load the directory metadata.", + "storage_details_error_access_denied": "You do not have permission to view these details.", + "storage_details_error_not_found": "Not found. It may have been moved or deleted.", "storage_details_error_calc_size": "Failed to calculate the directory size. Please try again.", - "storage_details_error_unknown": "Unknown error", "storage_details_error_no_body": "No response body", "storage_details_unnamed_rule": "(unnamed)", "storage_details_treemap_aria": "Treemap visualization of directory size composition", diff --git a/src/architecture-tests/i18n-compliance.spec.ts b/src/architecture-tests/i18n-compliance.spec.ts index d982ae8e..4f15aee4 100644 --- a/src/architecture-tests/i18n-compliance.spec.ts +++ b/src/architecture-tests/i18n-compliance.spec.ts @@ -79,10 +79,6 @@ const KNOWN_UNUSED_KEYS = [ 'storage_details_tree_visualization', 'storage_details_lifecycle_rule_id', 'storage_details_lifecycle_status', - 'storage_details_error_not_connected', - 'storage_details_error_fetch_file', - 'storage_details_error_fetch_bucket', - 'storage_details_error_fetch_dir_meta', 'storage_details_tab_overview', 'storage_details_tab_lifecycle', 'storage_details_bucket_name', diff --git a/src/hooks.server.ts b/src/hooks.server.ts index aa317dc0..05575211 100644 --- a/src/hooks.server.ts +++ b/src/hooks.server.ts @@ -11,6 +11,7 @@ import { getConnectionFromHeader } from '$lib/server/storage/connection.js'; import { STORAGE_CONNECTION_ID_HEADER } from '$lib/storage/connection-id-header.js'; import { storageBrowserEnabled } from '$lib/server/feature-flags.js'; import { storageEncryptionKey } from '$lib/server/storage/encryption-key.js'; +import { notConnected } from '$lib/server/storage/not-connected.js'; import { runMigrations } from '$lib/server/migrate.js'; // Allow self-signed TLS certificates in development (e.g. local Trino with self-signed certs). @@ -102,7 +103,7 @@ const handleStorageConnection: Handle = async ({ event, resolve }) => { !event.route.id?.startsWith('/(app)/api/storage/download/manifests/[manifestId]/') && event.route.id !== '/(app)/api/storage/download/manifests'; if (event.locals.storageConfig === null && requiresConnectionHeader) { - throw error(401, 'No storage connection configured'); + notConnected(); } return resolve(event); }; diff --git a/src/lib/components/storage/modals/DetailsModal.svelte b/src/lib/components/storage/modals/DetailsModal.svelte index 9207ec6d..a09f13b8 100644 --- a/src/lib/components/storage/modals/DetailsModal.svelte +++ b/src/lib/components/storage/modals/DetailsModal.svelte @@ -8,7 +8,7 @@ import IconBucket from '$lib/components/storage/shared/BucketIcon.svelte'; import * as m from '$lib/paraglide/messages.js'; import { getStorageState } from '$lib/storage/context.js'; - import { StorageError } from '$lib/storage/errors.js'; + import { getDetailsErrorMessage } from '$lib/storage/errors.js'; import type { FileDetails as FileDetailsType } from '$lib/storage/details-types.js'; import FileDetails from './details/FileDetails.svelte'; import DirectoryDetails from './details/DirectoryDetails.svelte'; @@ -54,11 +54,7 @@ try { fileDetails = await storage.api.details({ bucket, key }); } catch (err) { - if (err instanceof StorageError) { - loadError = err.message; - } else { - loadError = err instanceof Error ? err.message : m.storage_details_error_unknown(); - } + loadError = getDetailsErrorMessage(err, m.storage_details_error_fetch_file()); } finally { loading = false; } diff --git a/src/lib/components/storage/modals/PreviewModal.svelte b/src/lib/components/storage/modals/PreviewModal.svelte index decb11bd..aa703979 100644 --- a/src/lib/components/storage/modals/PreviewModal.svelte +++ b/src/lib/components/storage/modals/PreviewModal.svelte @@ -616,7 +616,7 @@ m.storage_editor_too_large({ limit: formatFileSize(maxEditableFileSize) }) ); } else { - addToast('error', err.message || m.storage_editor_error()); + addToast('error', m.storage_editor_error()); } } else { addToast('error', m.storage_editor_error()); diff --git a/src/lib/components/storage/modals/details/BucketDetails.svelte b/src/lib/components/storage/modals/details/BucketDetails.svelte index 14751b62..fed13b19 100644 --- a/src/lib/components/storage/modals/details/BucketDetails.svelte +++ b/src/lib/components/storage/modals/details/BucketDetails.svelte @@ -5,7 +5,7 @@ import IconPerson from 'virtual:icons/material-symbols/person'; import IconKey from 'virtual:icons/material-symbols/key'; import { getStorageState } from '$lib/storage/context.js'; - import { StorageError } from '$lib/storage/errors.js'; + import { getDetailsErrorMessage } from '$lib/storage/errors.js'; import type { BucketDetails as BucketDetailsType } from '$lib/storage/details-types.js'; interface Props { @@ -32,11 +32,7 @@ if (!cancelled) details = result; } catch (err) { if (cancelled) return; - if (err instanceof StorageError) { - error = err.message; - } else { - error = err instanceof Error ? err.message : m.storage_details_error_unknown(); - } + error = getDetailsErrorMessage(err, m.storage_details_error_fetch_bucket()); } finally { if (!cancelled) loading = false; } diff --git a/src/lib/components/storage/modals/details/DirectoryDetails.svelte b/src/lib/components/storage/modals/details/DirectoryDetails.svelte index cad14e71..d5d4240f 100644 --- a/src/lib/components/storage/modals/details/DirectoryDetails.svelte +++ b/src/lib/components/storage/modals/details/DirectoryDetails.svelte @@ -4,7 +4,7 @@ import IconWarning from 'virtual:icons/material-symbols/warning'; import { formatFileSize, keyToName } from '$lib/storage/utils.js'; import { getStorageState } from '$lib/storage/context.js'; - import { StorageError } from '$lib/storage/errors.js'; + import { getDetailsErrorMessage } from '$lib/storage/errors.js'; import type { DirectorySizeEvent, DirectoryMetadata, @@ -54,11 +54,7 @@ if (bucket === requestedBucket && prefix === requestedPrefix) meta = result; } catch (err) { if (bucket !== requestedBucket || prefix !== requestedPrefix) return; - if (err instanceof StorageError) { - metaError = err.message; - } else { - metaError = err instanceof Error ? err.message : m.storage_details_error_unknown(); - } + metaError = getDetailsErrorMessage(err, m.storage_details_error_fetch_dir_meta()); } } @@ -105,7 +101,7 @@ } else if (event.type === 'complete') { result = event; } else if (event.type === 'error') { - error = event.message; + error = m.storage_details_error_calc_size(); } } catch { // skip malformed events diff --git a/src/lib/server/storage/connection.test.ts b/src/lib/server/storage/connection.test.ts index fc77eade..51862406 100644 --- a/src/lib/server/storage/connection.test.ts +++ b/src/lib/server/storage/connection.test.ts @@ -67,7 +67,8 @@ describe('getConnectionFromHeader', () => { it('rejects an unknown connection', async () => { selectRows([]); await expect(getConnectionFromHeader(request(), 'user-id')).rejects.toMatchObject({ - status: 401 + status: 409, + body: { reason: 'not-connected' } }); }); diff --git a/src/lib/server/storage/connection.ts b/src/lib/server/storage/connection.ts index bebcc45c..571a0526 100644 --- a/src/lib/server/storage/connection.ts +++ b/src/lib/server/storage/connection.ts @@ -5,6 +5,7 @@ import { db } from '$lib/server/db.js'; import { userStorageConnections } from '$lib/server/schema.js'; import { decrypt } from './encryption.js'; import { storageEncryptionKey } from './encryption-key.js'; +import { notConnected } from './not-connected.js'; import type { S3ConnectionConfig } from './types.js'; import { logger } from '$lib/server/logging'; import { STORAGE_CONNECTION_ID_HEADER } from '$lib/storage/connection-id-header.js'; @@ -27,7 +28,7 @@ const AdditionalBucketsSchema = z.array(z.string()); * {@link LAST_USED_TOUCH_INTERVAL_MS}, so not every storage request hits the database. * * Returns `null` if the header is absent. - * Throws 401 if the connection is not found or does not belong to the user. + * Throws 409 (`notConnected`) if the connection is not found or does not belong to the user. * Throws 500 if decryption fails or the payload does not match the stored schema. */ export async function getConnectionFromHeader( @@ -48,7 +49,7 @@ export async function getConnectionFromHeader( if (rows.length === 0) { log.warn({ connection_id: connectionId }, 'storage connection not found or unauthorised'); - throw error(401, 'Storage connection not found'); + notConnected('Storage connection not found'); } const row = rows[0]; diff --git a/src/lib/server/storage/not-connected.ts b/src/lib/server/storage/not-connected.ts new file mode 100644 index 00000000..44e28364 --- /dev/null +++ b/src/lib/server/storage/not-connected.ts @@ -0,0 +1,10 @@ +import { error } from '@sveltejs/kit'; + +/** + * Reject a storage request that has no usable storage connection. A 409 with + * `reason: 'not-connected'` keeps it apart from a 401, which means the session + * expired and makes the client check the session. + */ +export function notConnected(message = 'No storage connection configured'): never { + error(409, { message, reason: 'not-connected' }); +} diff --git a/src/lib/server/storage/request-context.ts b/src/lib/server/storage/request-context.ts index bb9d0c5c..7f41f149 100644 --- a/src/lib/server/storage/request-context.ts +++ b/src/lib/server/storage/request-context.ts @@ -2,6 +2,7 @@ import { error } from '@sveltejs/kit'; import type { RequestEvent } from '@sveltejs/kit'; import { getProvider } from './utils.js'; import { withStorageHttpErrors } from './wrap-provider.js'; +import { notConnected } from './not-connected.js'; import type { StorageProvider } from './provider.js'; import { STORAGE_CONNECTION_ID_HEADER } from '$lib/storage/connection-id-header.js'; import { StorageObjectKeySchema } from '$lib/storage/schemas.js'; @@ -23,14 +24,14 @@ export function requireObjectKey(event: RequestEvent): string { export function requireConfig(event: RequestEvent) { const config = event.locals.storageConfig; - if (!config) throw error(401, 'No storage connection configured'); + if (!config) notConnected(); return config; } /** The connection id is captured for a manifest, never its decrypted config. */ export function requireStorageConnectionId(event: RequestEvent): string { const connectionId = event.request.headers.get(STORAGE_CONNECTION_ID_HEADER); - if (!connectionId) throw error(401, 'No storage connection configured'); + if (!connectionId) notConnected(); return connectionId; } diff --git a/src/lib/storage/archive.svelte.ts b/src/lib/storage/archive.svelte.ts index fb746e9a..6b5b528d 100644 --- a/src/lib/storage/archive.svelte.ts +++ b/src/lib/storage/archive.svelte.ts @@ -80,7 +80,7 @@ export class ArchiveState { try { await this._fetchArchiveListing(); - } catch (err) { + } catch { if (this._archiveNestedPath) { this._archiveNestedPath = null; } else { @@ -89,7 +89,7 @@ export class ArchiveState { } this.archivePrefix = ''; this.archiveLoading = false; - addToast('error', err instanceof Error ? err.message : m.storage_archive_open_error()); + addToast('error', m.storage_archive_open_error()); } } @@ -102,9 +102,9 @@ export class ArchiveState { try { await this._fetchArchiveListing(); - } catch (err) { + } catch { this.archiveLoading = false; - addToast('error', err instanceof Error ? err.message : m.storage_archive_open_error()); + addToast('error', m.storage_archive_open_error()); } } diff --git a/src/lib/storage/errors.spec.ts b/src/lib/storage/errors.spec.ts index f7b83c3e..cb284c4e 100644 --- a/src/lib/storage/errors.spec.ts +++ b/src/lib/storage/errors.spec.ts @@ -1,5 +1,10 @@ import { describe, it, expect, vi } from 'vitest'; -import { StorageError, getActionErrorMessage, getActionErrorMessageForCode } from './errors.js'; +import { + StorageError, + getActionErrorMessage, + getActionErrorMessageForCode, + getDetailsErrorMessage +} from './errors.js'; const mockMessages = vi.hoisted(() => ({ storage_download_error_not_connected: () => 'No storage connection configured', @@ -8,7 +13,10 @@ const mockMessages = vi.hoisted(() => ({ storage_download_error_server_error: () => 'Server error', storage_upload_error_no_such_bucket: () => 'Bucket not found', storage_upload_error_invalid_part: () => 'Invalid part', - storage_download_error_unknown: () => 'Unknown error' + storage_download_error_unknown: () => 'Unknown error', + storage_details_error_not_connected: () => 'Details: not connected', + storage_details_error_access_denied: () => 'Details: access denied', + storage_details_error_not_found: () => 'Details: not found' })); vi.mock('$lib/paraglide/messages.js', () => mockMessages); @@ -65,3 +73,20 @@ describe('getActionErrorMessageForCode', () => { expect(getActionErrorMessageForCode('unknown')).toBe('Unknown error'); }); }); + +describe('getDetailsErrorMessage', () => { + it.each([ + ['not_connected', 'Details: not connected'], + ['access_denied', 'Details: access denied'], + ['not_found', 'Details: not found'], + ['server_error', 'fallback'] + ])('maps %s', (code, expected) => { + expect(getDetailsErrorMessage(new StorageError(code, 'raw server text'), 'fallback')).toBe( + expected + ); + }); + + it('uses the fallback for errors that are not StorageErrors', () => { + expect(getDetailsErrorMessage(new TypeError('Failed to fetch'), 'fallback')).toBe('fallback'); + }); +}); diff --git a/src/lib/storage/errors.ts b/src/lib/storage/errors.ts index a8676bed..927e8cdc 100644 --- a/src/lib/storage/errors.ts +++ b/src/lib/storage/errors.ts @@ -48,3 +48,18 @@ export function getActionErrorMessage(err: StorageError): string { export function getActionErrorMessageForCode(code: string): string { return getActionErrorMessage(new StorageError(code, '')); } + +/** Translated message for a failed details request; `fallback` covers all other failures. */ +export function getDetailsErrorMessage(err: unknown, fallback: string): string { + if (!(err instanceof StorageError)) return fallback; + switch (err.code) { + case 'not_connected': + return m.storage_details_error_not_connected(); + case 'access_denied': + return m.storage_details_error_access_denied(); + case 'not_found': + return m.storage_details_error_not_found(); + default: + return fallback; + } +} diff --git a/src/lib/storage/storage-fetch.spec.ts b/src/lib/storage/storage-fetch.spec.ts index 55f51612..6c21c7f1 100644 --- a/src/lib/storage/storage-fetch.spec.ts +++ b/src/lib/storage/storage-fetch.spec.ts @@ -18,10 +18,6 @@ beforeEach(() => { }); describe('mapStatusToCode', () => { - it('maps 401 → not_connected', () => { - expect(mapStatusToCode(401)).toBe('not_connected'); - }); - it('maps 403 → access_denied', () => { expect(mapStatusToCode(403)).toBe('access_denied'); }); @@ -150,14 +146,22 @@ describe('createStorageFetch', () => { }); }); - it('throws StorageError on 401 with not_connected code', async () => { + it.each([ + [ + 'a 409 with reason not-connected', + 409, + { message: 'x', reason: 'not-connected' }, + 'not_connected' + ], + ['an edit conflict', 409, { message: 'Changed on the server' }, 'conflict'], + ['an expired session', 401, { message: 'Authentication required' }, 'unknown'] + ])('maps %s', async (_label, status, body, code) => { const storageFetch = createStorageFetch(() => 'conn-1'); - vi.spyOn(globalThis, 'fetch').mockResolvedValue(makeResponse({ status: 401 })); + vi.spyOn(globalThis, 'fetch').mockResolvedValue( + makeResponse({ status, body: JSON.stringify(body) }) + ); - await expect(storageFetch('/api/test')).rejects.toThrow(StorageError); - await expect(storageFetch('/api/test')).rejects.toMatchObject({ - code: 'not_connected' - }); + await expect(storageFetch('/api/test')).rejects.toMatchObject({ code }); }); it('throws StorageError on 400 with unknown code', async () => { diff --git a/src/lib/storage/storage-fetch.ts b/src/lib/storage/storage-fetch.ts index 195d7cd8..34ee924b 100644 --- a/src/lib/storage/storage-fetch.ts +++ b/src/lib/storage/storage-fetch.ts @@ -23,7 +23,6 @@ import { StorageError } from '$lib/storage/errors.js'; * `state.svelte.ts`, `download.ts`, and `upload.ts`. */ export function mapStatusToCode(status: number): string { - if (status === 401) return 'not_connected'; if (status === 403) return 'access_denied'; if (status === 404) return 'not_found'; if (status === 409) return 'conflict'; @@ -32,6 +31,22 @@ export function mapStatusToCode(status: number): string { return 'unknown'; } +/** + * Whether an error body says the request has no usable storage connection. + * The server answers that with 409 and `reason: 'not-connected'`, keeping 401 + * for an expired session. + */ +export function isNotConnectedBody(body: unknown): boolean { + return ( + typeof body === 'object' && body !== null && 'reason' in body && body.reason === 'not-connected' + ); +} + +/** StorageError code for a failed response, given its parsed body. */ +export function errorCodeFor(status: number, body: unknown): string { + return isNotConnectedBody(body) ? 'not_connected' : mapStatusToCode(status); +} + /** * Create a `storageFetch` function bound to a connection-ID getter. * @@ -55,7 +70,11 @@ export function createStorageFetch( const response = await fetch(path, { ...init, headers }); if (!response.ok) { - const code = mapStatusToCode(response.status); + const body: unknown = await response + .clone() + .json() + .catch(() => null); + const code = errorCodeFor(response.status, body); const message = (await readErrorMessage(response)) ?? `Request failed with status ${response.status}`; throw new StorageError(code, message); diff --git a/src/lib/storage/upload.ts b/src/lib/storage/upload.ts index c7fe2a2e..10c01e75 100644 --- a/src/lib/storage/upload.ts +++ b/src/lib/storage/upload.ts @@ -14,7 +14,7 @@ import { STORAGE_CONNECTION_ID_HEADER } from '$lib/storage/connection-id-header.js'; import { StorageError } from '$lib/storage/errors.js'; -import { createStorageFetch, mapStatusToCode } from '$lib/storage/storage-fetch.js'; +import { createStorageFetch, errorCodeFor } from '$lib/storage/storage-fetch.js'; /** * Check whether an object with the given key already exists in the bucket. @@ -84,7 +84,13 @@ export function uploadFile( resolve(); return; } - const code = mapStatusToCode(xhr.status); + let body: unknown = null; + try { + body = JSON.parse(xhr.responseText); + } catch { + // Not a JSON error body, e.g. a proxy's error page. + } + const code = errorCodeFor(xhr.status, body); reject(new StorageError(code, `Upload failed with status ${xhr.status}`)); }); diff --git a/src/routes/(app)/api/storage/archive/extract/+server.ts b/src/routes/(app)/api/storage/archive/extract/+server.ts index 268a190b..3ddf4442 100644 --- a/src/routes/(app)/api/storage/archive/extract/+server.ts +++ b/src/routes/(app)/api/storage/archive/extract/+server.ts @@ -1,4 +1,5 @@ import { error } from '@sveltejs/kit'; +import { notConnected } from '$lib/server/storage/not-connected.js'; import { ArchiveCacheFullError, extractArchiveEntry, @@ -48,7 +49,7 @@ export const GET: RequestHandler = async (event) => { const config = locals.storageConfig; const connectionId = locals.storageConnectionId; if (!config || !connectionId) { - throw error(401, 'No storage connection configured'); + notConnected(); } locals.logger.debug( diff --git a/src/routes/(app)/api/storage/archive/listing/+server.ts b/src/routes/(app)/api/storage/archive/listing/+server.ts index 2e3088dd..e125c09c 100644 --- a/src/routes/(app)/api/storage/archive/listing/+server.ts +++ b/src/routes/(app)/api/storage/archive/listing/+server.ts @@ -1,4 +1,5 @@ import { error } from '@sveltejs/kit'; +import { notConnected } from '$lib/server/storage/not-connected.js'; import { ArchiveCacheFullError, listArchiveContents } from '$lib/server/storage/archive.js'; import { getProvider } from '$lib/server/storage/utils.js'; import { requireObjectKey } from '$lib/server/storage/request-context.js'; @@ -33,7 +34,7 @@ export const GET: RequestHandler = async (event) => { const config = locals.storageConfig; const connectionId = locals.storageConnectionId; if (!config || !connectionId) { - throw error(401, 'No storage connection configured'); + notConnected(); } locals.logger.debug( diff --git a/src/routes/(app)/api/storage/buckets/+server.ts b/src/routes/(app)/api/storage/buckets/+server.ts index 255dfc90..a3b3c9ae 100644 --- a/src/routes/(app)/api/storage/buckets/+server.ts +++ b/src/routes/(app)/api/storage/buckets/+server.ts @@ -1,4 +1,5 @@ -import { json, error } from '@sveltejs/kit'; +import { json } from '@sveltejs/kit'; +import { notConnected } from '$lib/server/storage/not-connected.js'; import type { RequestHandler } from './$types'; import type { BucketDetails } from '$lib/storage/details-types.js'; import { getConnectionProvider } from '$lib/server/storage/utils.js'; @@ -33,7 +34,7 @@ export const GET: RequestHandler = async (event) => { const config = event.locals.storageConfig; if (!config) { - throw error(401, 'No storage connection configured'); + notConnected(); } const listedBuckets = await withStorageHttpErrors(getConnectionProvider(config)).listContainers(); diff --git a/src/routes/(app)/storage/+layout.ts b/src/routes/(app)/storage/+layout.ts index 22e81c7a..a52cf79a 100644 --- a/src/routes/(app)/storage/+layout.ts +++ b/src/routes/(app)/storage/+layout.ts @@ -1,15 +1,18 @@ import { browser } from '$app/environment'; import { STORAGE_CONNECTION_ID_HEADER } from '$lib/storage/connection-id-header.js'; import { connectionStore } from '$lib/storage/connection-store.svelte.js'; +import { isNotConnectedBody } from '$lib/storage/storage-fetch.js'; import type { LayoutLoad } from './$types'; /** Why the active connection could not be opened; the connect form shows it. */ type BucketsError = 'access_denied' | 'unreachable' | null; -// 401: the session holds no usable connection, which is not an error. -function bucketsErrorFor(status: number): BucketsError { - if (status === 401) return null; - return status === 403 ? 'access_denied' : 'unreachable'; +// No usable connection is not an error: the connect form is all that is needed. +// A 401 means the session expired, which the session-expired modal reports. +async function bucketsErrorFor(res: Response): Promise { + const body: unknown = await res.json().catch(() => null); + if (res.status === 401 || isNotConnectedBody(body)) return null; + return res.status === 403 ? 'access_denied' : 'unreachable'; } const DISCONNECTED = { @@ -75,7 +78,7 @@ export const load: LayoutLoad = async ({ fetch, data }) => { if (!res.ok) { return { ...DISCONNECTED, - bucketsError: bucketsErrorFor(res.status), + bucketsError: await bucketsErrorFor(res), connections, hydrating: false as const, hasActiveConnection: false as const diff --git a/src/routes/(app)/storage/browse/[connection]/[bucket]/[...prefix]/+page.ts b/src/routes/(app)/storage/browse/[connection]/[bucket]/[...prefix]/+page.ts index e24e6e33..8a1535c0 100644 --- a/src/routes/(app)/storage/browse/[connection]/[bucket]/[...prefix]/+page.ts +++ b/src/routes/(app)/storage/browse/[connection]/[bucket]/[...prefix]/+page.ts @@ -5,6 +5,7 @@ import { STORAGE_CONNECTION_ID_HEADER } from '$lib/storage/connection-id-header. import * as m from '$lib/paraglide/messages.js'; import type { PageLoad } from './$types'; import type { StoragePage } from '$lib/storage/types.js'; +import { isNotConnectedBody } from '$lib/storage/storage-fetch.js'; const EMPTY_PAGE: StoragePage = { objects: [], @@ -13,6 +14,15 @@ const EMPTY_PAGE: StoragePage = { pageSize: 25 }; +/** A session or connection problem sends the user back to the connect form. */ +async function throwListError(res: Response, bucket: string): Promise { + const body: unknown = await res.json().catch(() => null); + if (res.status === 401 || isNotConnectedBody(body)) throw redirect(303, '/storage'); + if (res.status === 403) throw error(403, m.storage_error_access_denied({ bucket })); + const message = (body as { message?: string } | null)?.message; + throw error(res.status, message ?? 'Failed to load objects'); +} + /** * Universal load: fetches the object list from the API. * @@ -54,12 +64,7 @@ export const load: PageLoad = async ({ fetch, url, data, parent }) => { throw err; } - if (!res.ok) { - if (res.status === 401) throw redirect(303, '/storage'); - if (res.status === 403) throw error(403, m.storage_error_access_denied({ bucket })); - const body = (await res.json().catch(() => ({}))) as { message?: string }; - throw error(res.status, body.message ?? 'Failed to load objects'); - } + if (!res.ok) await throwListError(res, bucket); return { connection, bucket, prefix, objects: EMPTY_PAGE, hydrating: true }; } @@ -74,12 +79,7 @@ export const load: PageLoad = async ({ fetch, url, data, parent }) => { headers: { [STORAGE_CONNECTION_ID_HEADER]: connectionId } }); - if (!res.ok) { - if (res.status === 401) throw redirect(303, '/storage'); - if (res.status === 403) throw error(403, m.storage_error_access_denied({ bucket })); - const body = (await res.json().catch(() => ({}))) as { message?: string }; - throw error(res.status, body.message ?? 'Failed to load objects'); - } + if (!res.ok) await throwListError(res, bucket); const objects = (await res.json()) as StoragePage; return { connection, bucket, prefix, objects, hydrating: false };