From ddd2a4078eec9f0fb73f2ad8e7fd8606fb2eedaa Mon Sep 17 00:00:00 2001 From: Luis Henrique Mulinari Date: Thu, 30 Jul 2026 17:07:13 -0300 Subject: [PATCH 1/4] Improve live backup copy error messages - Extend parseApiError to pull the message out of an Apollo ServerError HTTP response body (bodyText), covering both {status,message} and GraphQL {errors:[{message}]} payloads, and accept an unknown error so callers don't need to cast. - Use parseApiError in the live backup copy catch block so users see the specific API message (e.g. "Request body is too large") instead of a generic "Received status code 413". - Reject live backup config files larger than 500 KB with a UserError suggesting the --wpcli-command option. - Throw UserError instead of Error for config validation failures. --- __tests__/lib/utils.js | 71 +++++++++++++++++++++++++++++++++++++- src/commands/export-sql.ts | 31 +++++++++++++---- src/lib/utils.ts | 53 ++++++++++++++++++++++++---- 3 files changed, 141 insertions(+), 14 deletions(-) diff --git a/__tests__/lib/utils.js b/__tests__/lib/utils.js index 1edcfc51e..6c31c4984 100644 --- a/__tests__/lib/utils.js +++ b/__tests__/lib/utils.js @@ -1,4 +1,4 @@ -import { splitKeyValueString } from '../../src/lib/utils'; +import { parseApiError, splitKeyValueString } from '../../src/lib/utils'; describe( 'splitKeyValueString', () => { it.each( [ @@ -13,3 +13,72 @@ describe( 'splitKeyValueString', () => { expect( splitKeyValueString( input ) ).toEqual( expected ); } ); } ); + +describe( 'parseApiError', () => { + it( 'extracts the message from an HTTP response body (bodyText)', () => { + const error = Object.assign( new Error( 'Response not successful: Received status code 413' ), { + bodyText: '{"status":"error","message":"Request body is too large"}', + } ); + + expect( parseApiError( error ) ).toBe( 'Request body is too large' ); + } ); + + it( 'extracts a GraphQL error message from a bodyText errors array', () => { + const error = Object.assign( new Error( 'Response not successful: Received status code 400' ), { + bodyText: JSON.stringify( { + errors: [ { message: 'Cannot query field "npmToken" on type "BuildConfiguration".' } ], + } ), + } ); + + expect( parseApiError( error ) ).toBe( + 'Cannot query field "npmToken" on type "BuildConfiguration".' + ); + } ); + + it( 'extracts the first message from a bodyText errors array', () => { + const error = Object.assign( new Error( 'Bad Request' ), { + bodyText: JSON.stringify( { + errors: [ + { message: 'Field "provider" of required type was not provided.' }, + { message: 'Field "providerXX" is not defined. Did you mean "provider"?' }, + ], + } ), + } ); + + expect( parseApiError( error ) ).toBe( 'Field "provider" of required type was not provided.' ); + } ); + + it( 'extracts a networkError message', () => { + const error = { networkError: { message: 'network is down' } }; + + expect( parseApiError( error ) ).toBe( 'network is down' ); + } ); + + it( 'extracts a graphQLErrors message', () => { + const error = { graphQLErrors: [ { message: 'BAD_REQUEST message' } ] }; + + expect( parseApiError( error ) ).toBe( 'BAD_REQUEST message' ); + } ); + + it( 'falls back to the error message when bodyText is not JSON', () => { + const error = Object.assign( new Error( 'Boom' ), { bodyText: 'not json' } ); + + expect( parseApiError( error ) ).toBe( 'Boom' ); + } ); + + it( 'falls back to the error message when the body has no message field', () => { + const error = Object.assign( new Error( 'Boom' ), { + bodyText: '{"status":"error"}', + } ); + + expect( parseApiError( error ) ).toBe( 'Boom' ); + } ); + + it( 'returns the message for a plain Error', () => { + expect( parseApiError( new Error( 'Something failed' ) ) ).toBe( 'Something failed' ); + } ); + + it( 'returns null for unknown error shapes', () => { + expect( parseApiError( { foo: 'bar' } ) ).toBeNull(); + } ); +} ); diff --git a/src/commands/export-sql.ts b/src/commands/export-sql.ts index 607d92151..9e32e73b2 100644 --- a/src/commands/export-sql.ts +++ b/src/commands/export-sql.ts @@ -29,10 +29,14 @@ import { BackupLiveCopyType, } from '../lib/live-backup-copy'; import { retry } from '../lib/retry'; -import { getAbsolutePath, pollUntil } from '../lib/utils'; +import UserError from '../lib/user-error'; +import { getAbsolutePath, pollUntil, parseApiError } from '../lib/utils'; const EXPORT_SQL_PROGRESS_POLL_INTERVAL = 1000; +// Maximum allowed size for a live backup copy configuration file (500 KB). +const MAX_LIVE_BACKUP_CONFIG_SIZE_BYTES = 500 * 1024; + const BACKUP_AND_JOB_STATUS_QUERY = gql` query AppBackupAndJobStatus($appId: Int!, $envId: Int!) { app(id: $appId) { @@ -569,11 +573,11 @@ export class ExportSQLCommand { private async generateLiveBackupCopy() { if ( ! this.liveBackupCopyCLIOptions ) { - throw new Error( 'Configuration file is required for live backup copy.' ); + throw new UserError( 'Configuration file is required for live backup copy.' ); } if ( ! this.app.id || ! this.env.id ) { - throw new Error( 'App ID and Environment ID are required to start live backup copy.' ); + throw new UserError( 'App ID and Environment ID are required to start live backup copy.' ); } const config = this.getLiveBackupConfigFromCLIOptions(); @@ -601,7 +605,7 @@ export class ExportSQLCommand { return result; } catch ( err ) { - const message = err instanceof Error ? err.message : 'Unknown error'; + const message = parseApiError( err ) ?? 'Unknown error'; await this.track( 'error', { error_type: 'live_backup_copy', @@ -646,7 +650,18 @@ export class ExportSQLCommand { private loadLiveBackupCopyConfig( configFile: string ): DBLiveCopyConfig { if ( ! fs.existsSync( configFile ) ) { - throw new Error( `Configuration file not found: ${ configFile }` ); + throw new UserError( `Configuration file not found: ${ configFile }` ); + } + + const { size } = fs.statSync( configFile ); + if ( size > MAX_LIVE_BACKUP_CONFIG_SIZE_BYTES ) { + throw new UserError( + `Configuration file is too large (${ formatBytes( + size + ) }); the maximum allowed size is ${ formatBytes( + MAX_LIVE_BACKUP_CONFIG_SIZE_BYTES + ) }. For large or complex exports, use the \`--wpcli-command\` option instead.` + ); } try { @@ -654,10 +669,12 @@ export class ExportSQLCommand { } catch ( err ) { const errMessage = err instanceof Error ? err.message : 'Unknown error'; if ( err instanceof SyntaxError ) { - throw new Error( `Invalid JSON in configuration file: ${ configFile } - ${ errMessage }` ); + throw new UserError( + `Invalid JSON in configuration file: ${ configFile } - ${ errMessage }` + ); } - throw new Error( `Error reading configuration file: ${ configFile } - ${ errMessage }` ); + throw new UserError( `Error reading configuration file: ${ configFile } - ${ errMessage }` ); } } } diff --git a/src/lib/utils.ts b/src/lib/utils.ts index 2c7adedfc..d0bf19af2 100644 --- a/src/lib/utils.ts +++ b/src/lib/utils.ts @@ -79,17 +79,49 @@ export function getAbsolutePath( filePath: string ): string { } /** - * Parse error object and return probable error. - * - * @param {Error} Error object + * Extract a message from a raw HTTP response body (Apollo's `ServerError.bodyText`). * - * @return {string|null} Error string when error was found, otherwise null. + * The body may be either `{"status":"error","message":"..."}` (e.g. a 413 whose + * top-level message is only `Received status code 413`) or a GraphQL + * `{"errors":[{"message":"..."}]}` payload. */ -export function parseApiError( err: { +function parseJSONBodyErrorMessage( bodyText: string ): string | null { + try { + const body = JSON.parse( bodyText ) as { + message?: string; + errors?: { message?: string }[]; + }; + + if ( body?.errors?.[ 0 ]?.message ) { + return body.errors[ 0 ].message; + } + + if ( body?.message ) { + return body.message; + } + } catch { + // Body wasn't JSON; nothing to extract. + } + + return null; +} + +interface ApiError { + bodyText?: string; networkError?: { message?: string }; message?: string; graphQLErrors?: { message?: string }[]; -} ): string | null { +} + +/** + * Parse error object and return probable error. + * + * Accepts either a known API error shape or an `unknown` value (e.g. a caught + * error), so callers do not need to cast before passing it in. + */ +export function parseApiError( error: unknown ): string | null { + const err = error as ApiError; + if ( err?.networkError?.message ) { return err?.networkError?.message; } @@ -98,6 +130,15 @@ export function parseApiError( err: { return err.graphQLErrors[ 0 ].message; } + // Apollo's `ServerError` exposes the raw HTTP response body as `bodyText`, + // which often holds a more specific message than the generic top-level one. + if ( typeof err?.bodyText === 'string' ) { + const bodyTextMessage = parseJSONBodyErrorMessage( err.bodyText ); + if ( bodyTextMessage ) { + return bodyTextMessage; + } + } + if ( err?.message ) { return err?.message; } From 272d399e6b7de39cb713ef80dd9ec0140dc1893e Mon Sep 17 00:00:00 2001 From: Luis Henrique Mulinari Date: Fri, 31 Jul 2026 14:08:50 -0300 Subject: [PATCH 2/4] Fix file system race in loadLiveBackupCopyConfig Read the config file exactly once with readFileSync and derive existence (via ENOENT), size, and contents from that single read, instead of calling existsSync/statSync before readFileSync. This removes the time-of-check/time-of-use race flagged by CodeQL. --- src/commands/export-sql.ts | 18 ++++++++++++++---- 1 file changed, 14 insertions(+), 4 deletions(-) diff --git a/src/commands/export-sql.ts b/src/commands/export-sql.ts index 9e32e73b2..980df86a7 100644 --- a/src/commands/export-sql.ts +++ b/src/commands/export-sql.ts @@ -649,11 +649,21 @@ export class ExportSQLCommand { } private loadLiveBackupCopyConfig( configFile: string ): DBLiveCopyConfig { - if ( ! fs.existsSync( configFile ) ) { - throw new UserError( `Configuration file not found: ${ configFile }` ); + // Read the file exactly once and derive existence, size, and contents from + // that single read to avoid a time-of-check/time-of-use race condition. + let fileContents: string; + try { + fileContents = fs.readFileSync( configFile, 'utf-8' ); + } catch ( err ) { + if ( ( err as NodeJS.ErrnoException )?.code === 'ENOENT' ) { + throw new UserError( `Configuration file not found: ${ configFile }` ); + } + + const errMessage = err instanceof Error ? err.message : 'Unknown error'; + throw new UserError( `Error reading configuration file: ${ configFile } - ${ errMessage }` ); } - const { size } = fs.statSync( configFile ); + const size = Buffer.byteLength( fileContents, 'utf-8' ); if ( size > MAX_LIVE_BACKUP_CONFIG_SIZE_BYTES ) { throw new UserError( `Configuration file is too large (${ formatBytes( @@ -665,7 +675,7 @@ export class ExportSQLCommand { } try { - return JSON.parse( fs.readFileSync( configFile, 'utf-8' ) ) as DBLiveCopyConfig; + return JSON.parse( fileContents ) as DBLiveCopyConfig; } catch ( err ) { const errMessage = err instanceof Error ? err.message : 'Unknown error'; if ( err instanceof SyntaxError ) { From 91faa8881d70505b61e6f744f5a9584987a5297b Mon Sep 17 00:00:00 2001 From: Luis Henrique Mulinari Date: Fri, 31 Jul 2026 14:10:30 -0300 Subject: [PATCH 3/4] Linting --- src/bin/vip-app-deploy-token-generate.ts | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/src/bin/vip-app-deploy-token-generate.ts b/src/bin/vip-app-deploy-token-generate.ts index 96219cfaf..d5840a56d 100644 --- a/src/bin/vip-app-deploy-token-generate.ts +++ b/src/bin/vip-app-deploy-token-generate.ts @@ -90,8 +90,7 @@ export async function appDeployTokenGenerateCmd( }, } ); } catch ( err ) { - const message = - parseApiError( err as Parameters< typeof parseApiError >[ 0 ] ) ?? 'Unknown error'; + const message = parseApiError( err ) ?? 'Unknown error'; await trackerFn( 'error', { error: message } ); exit.withError( `Failed to generate deploy token: ${ message }` ); return; From 294fa3fd1d9b587eb820e1dd61418244c5f72081 Mon Sep 17 00:00:00 2001 From: Luis Henrique Mulinari Date: Fri, 31 Jul 2026 14:11:52 -0300 Subject: [PATCH 4/4] Simplify loadLiveBackupCopyConfig Read the config file once inside a single try/catch instead of the separate existsSync/statSync checks and nested error handling. Keeps the CodeQL race-condition fix but with far less branching. --- src/commands/export-sql.ts | 39 ++++++++++++++++---------------------- 1 file changed, 16 insertions(+), 23 deletions(-) diff --git a/src/commands/export-sql.ts b/src/commands/export-sql.ts index 980df86a7..3e31152cf 100644 --- a/src/commands/export-sql.ts +++ b/src/commands/export-sql.ts @@ -649,34 +649,27 @@ export class ExportSQLCommand { } private loadLiveBackupCopyConfig( configFile: string ): DBLiveCopyConfig { - // Read the file exactly once and derive existence, size, and contents from - // that single read to avoid a time-of-check/time-of-use race condition. - let fileContents: string; try { - fileContents = fs.readFileSync( configFile, 'utf-8' ); - } catch ( err ) { - if ( ( err as NodeJS.ErrnoException )?.code === 'ENOENT' ) { - throw new UserError( `Configuration file not found: ${ configFile }` ); - } - - const errMessage = err instanceof Error ? err.message : 'Unknown error'; - throw new UserError( `Error reading configuration file: ${ configFile } - ${ errMessage }` ); - } + // Read the file once to avoid a check-then-use race condition. + const fileContents = fs.readFileSync( configFile, 'utf-8' ); - const size = Buffer.byteLength( fileContents, 'utf-8' ); - if ( size > MAX_LIVE_BACKUP_CONFIG_SIZE_BYTES ) { - throw new UserError( - `Configuration file is too large (${ formatBytes( - size - ) }); the maximum allowed size is ${ formatBytes( - MAX_LIVE_BACKUP_CONFIG_SIZE_BYTES - ) }. For large or complex exports, use the \`--wpcli-command\` option instead.` - ); - } + const size = Buffer.byteLength( fileContents ); + if ( size > MAX_LIVE_BACKUP_CONFIG_SIZE_BYTES ) { + throw new UserError( + `Configuration file is too large (${ formatBytes( + size + ) }); the maximum allowed size is ${ formatBytes( + MAX_LIVE_BACKUP_CONFIG_SIZE_BYTES + ) }. For large or complex exports, use the \`--wpcli-command\` option instead.` + ); + } - try { return JSON.parse( fileContents ) as DBLiveCopyConfig; } catch ( err ) { + if ( err instanceof UserError ) { + throw err; + } + const errMessage = err instanceof Error ? err.message : 'Unknown error'; if ( err instanceof SyntaxError ) { throw new UserError(