diff --git a/cli/src/bundle/partial.ts b/cli/src/bundle/partial.ts index 9edb4643b7..a708e9375a 100644 --- a/cli/src/bundle/partial.ts +++ b/cli/src/bundle/partial.ts @@ -15,7 +15,7 @@ import * as tus from 'tus-js-client' import { buildCliRequestHeaders } from '../analytics/cli-headers' import { encryptChecksum, encryptChecksumV3, encryptSource } from '../api/crypto' import { CliUserError } from '../shared/cli-user-error' -import { appAddHintMessage, BROTLI_MIN_UPDATER_VERSION_V5, BROTLI_MIN_UPDATER_VERSION_V6, BROTLI_MIN_UPDATER_VERSION_V7, deltaManifestTooLargeMessage, findRoot, generateManifest, getContentType, getInstalledVersion, getLocalConfig, isAppNotFoundError, isDeprecatedPluginVersion, MAX_MANIFEST_ENTRIES, sendEvent, TUS_UPLOAD_RETRY_DELAYS } from '../utils' +import { BROTLI_MIN_UPDATER_VERSION_V5, BROTLI_MIN_UPDATER_VERSION_V6, BROTLI_MIN_UPDATER_VERSION_V7, buildTusUploadError, deltaManifestTooLargeMessage, findRoot, generateManifest, getContentType, getInstalledVersion, getLocalConfig, isDeprecatedPluginVersion, MAX_MANIFEST_ENTRIES, sendEvent, TUS_UPLOAD_RETRY_DELAYS } from '../utils' import { getUploadReporter } from './reporter' const log = { @@ -311,35 +311,13 @@ export async function uploadPartial( }, headers: buildCliRequestHeaders({ Authorization: apikey }), onError: (error) => { - const errorMessage = error.toString() - - // Turn the backend's `app_not_found` rejection into the actionable `app add` - // hint. Without this the raw tus error object escapes as an unhandled - // rejection instead of a clear user error. - if (isAppNotFoundError(error)) { - log.error(`Failed to upload ${filePathUnix}: ${errorMessage}`) - reject(new Error(appAddHintMessage(appId))) - return - } - - // Try to extract requestId from error message - let requestId: string | undefined - try { - // TUS errors often include response text in the format: "response text: {json}" - const responseTextMatch = errorMessage.match(/response text: (\{.*?\})/) - if (responseTextMatch && responseTextMatch[1]) { - const errorResponse = JSON.parse(responseTextMatch[1]) - requestId = errorResponse.moreInfo?.requestId - } - } - catch { - // Ignore JSON parse errors - } - - const requestIdSuffix = requestId ? ` [requestId: ${requestId}]` : '' - log.error(`Failed to upload ${filePathUnix}: ${errorMessage}${requestIdSuffix}`) - - reject(error) + // Reject a real Error carrying the HTTP status, backend message, and + // request id — the same shape as `uploadTUS`. Rejecting the raw tus + // blob leaked the URL and per-file object key into the message and + // dropped the status the error-tracking filter needs. + const uploadError = buildTusUploadError(error, appId) + log.error(`Failed to upload ${filePathUnix}: ${uploadError.message}`) + reject(uploadError) }, onProgress() { const percentage = ((uploadedFiles / totalFiles) * 100).toFixed(2) diff --git a/cli/src/posthog.ts b/cli/src/posthog.ts index a66cfb53dd..b90f3851b2 100644 --- a/cli/src/posthog.ts +++ b/cli/src/posthog.ts @@ -188,6 +188,7 @@ const EXPECTED_USER_ERROR_MARKERS = [ 'invalid apikey', 'no_key_provided', 'no key provided', + 'user_not_found', 'invalid api key or insufficient permissions', 'does not exist, run first', ] diff --git a/cli/src/utils.ts b/cli/src/utils.ts index 45eedfb8dd..059c6ba387 100644 --- a/cli/src/utils.ts +++ b/cli/src/utils.ts @@ -1689,6 +1689,46 @@ export function isAppNotFoundError(error: unknown): boolean { return `${responseBody ?? ''} ${message}`.includes('app_not_found') } +// Turn a tus upload failure into a real Error that carries the HTTP status, the +// backend error code/message, and the request id. Attaching `.status` lets the +// CLI error-tracking filter treat an auth failure (401) as an expected user +// error, and building a clean message keeps the raw tus blob — which embeds the +// upload URL and per-file object key — out of the message. +export function buildTusUploadError(error: unknown, appId: string): Error & { status?: number } { + // The backend rejects unknown apps with `404 app_not_found`; surface the + // actionable `app add` hint instead of a raw tus error string. + if (isAppNotFoundError(error)) + return new Error(appAddHintMessage(appId)) + + if (error instanceof tus.DetailedError) { + const body = error.originalResponse?.getBody() + const status = error.originalResponse?.getStatus() + const url = error.originalRequest?.getURL() + + // Parse can throw on a non-JSON body (an HTML 502/504 page from a proxy), + // so keep it inside the try and fall back to the raw body, then the tus + // error message. + let backendMessage: string + let requestId: string | undefined + try { + const jsonBody = JSON.parse(body || '{"error": "unknown error"}') + backendMessage = jsonBody.status || jsonBody.error || jsonBody.message || 'unknown error' + requestId = jsonBody.moreInfo?.requestId + } + catch { + backendMessage = body || error.message + } + + const requestIdSuffix = requestId ? ` [requestId: ${requestId}]` : '' + const built = new Error(`TUS upload failed (status ${status ?? 'unknown'}, url ${url ?? 'unknown'}): ${backendMessage}${requestIdSuffix}`) as Error & { status?: number } + if (typeof status === 'number') + built.status = status + return built + } + + return new Error(`TUS upload failed: ${error instanceof Error ? (error.message || error.toString()) : String(error)}`) +} + export async function uploadTUS(apikey: string, data: Buffer, orgId: string, appId: string, name: string, spinner: UploadSpinner, localConfig: CapgoConfig, chunkSize: number): Promise { return new Promise((resolve, reject) => { sendEvent(apikey, { @@ -1719,35 +1759,7 @@ export async function uploadTUS(apikey: string, data: Buffer, orgId: string, app // Callback for errors which cannot be fixed using retries onError(error) { log.error(`Error uploading bundle: ${error.message}`) - // Turn the backend's `app_not_found` rejection into the actionable `app add` - // hint instead of leaking a raw tus error string to the user. - if (isAppNotFoundError(error)) { - reject(new Error(appAddHintMessage(appId))) - return - } - if (error instanceof tus.DetailedError) { - const body = error.originalResponse?.getBody() - const status = error.originalResponse?.getStatus() - const url = error.originalRequest?.getURL() - - // Parse can throw on a non-JSON body (an HTML 502/504 page from a proxy), - // so keep it inside the try and fall back to the raw body, then the tus - // error message. An empty body used to collapse to the literal - // "unknown error" and drop the status, URL, and body on the floor. - const errorMsg = (() => { - try { - const jsonBody = JSON.parse(body || '{"error": "unknown error"}') - return jsonBody.status || jsonBody.error || jsonBody.message || 'unknown error' - } - catch { - return body || error.message - } - })() - reject(new Error(`TUS upload failed (status ${status ?? 'unknown'}, url ${url ?? 'unknown'}): ${errorMsg}`)) - } - else { - reject(new Error(`TUS upload failed: ${error.message || error.toString()}`)) - } + reject(buildTusUploadError(error, appId)) }, // Callback for reporting upload progress onProgress(bytesUploaded, bytesTotal) { diff --git a/cli/test/test-posthog-exception.mjs b/cli/test/test-posthog-exception.mjs index ed890c0d93..95851ec298 100644 --- a/cli/test/test-posthog-exception.mjs +++ b/cli/test/test-posthog-exception.mjs @@ -232,6 +232,10 @@ try { assert.equal(isExpectedUserError(new Error('App com.example does not exist, run first `npx @capgo/cli app add com.example` to create it')), true) assert.equal(isExpectedUserError({ context: { status: 401 } }), true) assert.equal(isExpectedUserError({ status: 401 }), true) + // A 401 `user_not_found` from the files backend must be treated as an auth + // error, both via the marker and via the status attached to the thrown error. + assert.equal(isExpectedUserError(new Error('user_not_found')), true) + assert.equal(isExpectedUserError(Object.assign(new Error('TUS upload failed (status 401, url https://x): user_not_found'), { status: 401 })), true) assert.equal(isExpectedUserError(new Error('Cannot get organization id for app id com.example')), false) assert.equal(isExpectedUserError(new Error('boom')), false) assert.equal(shouldCapturePosthogException(new Error('invalid_apikey')), false) diff --git a/supabase/functions/_backend/files/files.ts b/supabase/functions/_backend/files/files.ts index 5724f6ac1a..7c68361246 100644 --- a/supabase/functions/_backend/files/files.ts +++ b/supabase/functions/_backend/files/files.ts @@ -908,7 +908,9 @@ async function checkWriteAppAccess(c: Context, next: Next) { app_id, capgkeyPrefix: capgkey ? capgkey.substring(0, 15) : 'missing', }) - throw new HTTPException(400, { + // 401, not 400, so a genuine missing user matches how `invalid_apikey` + // behaves on this route and the CLI treats it as an auth error. + throw new HTTPException(401, { res: c.json({ error: 'user_not_found', message: 'User not found for the provided API key', diff --git a/supabase/functions/_backend/utils/pg_files.ts b/supabase/functions/_backend/utils/pg_files.ts index b2c39fe1ec..32750b9579 100644 --- a/supabase/functions/_backend/utils/pg_files.ts +++ b/supabase/functions/_backend/utils/pg_files.ts @@ -37,7 +37,10 @@ export async function getUserIdFromApikey( } catch (e: unknown) { logPgError(c, 'getUserIdFromApikey', e) - return null + // A failed query is a backend problem, not a missing user. Rethrow so it + // surfaces as a 500 instead of a null result that the caller reports to the + // user as a bad API key. + throw e } }