Improve live backup copy error messages - #2988
Open
luismulinari wants to merge 4 commits into
Open
Conversation
- 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.
Contributor
Dependency Review✅ No vulnerabilities or license issues or OpenSSF Scorecard issues found.Scanned FilesNone |
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.
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.
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Surfaces the specific API error message to users during
vip export-sqllive backup copy, instead of a generic HTTP status message, and adds guardrails for oversized config files.Previously a failed live backup copy showed:
Now it shows the actual reason from the API response body:
Changes
parseApiError(src/lib/utils.ts): extended to pull the message out of an ApolloServerErrorHTTP response body (bodyText), covering both{"status":"error","message":"..."}and GraphQL{"errors":[{"message":"..."}]}payloads. The signature now acceptsunknownso callers don't need to cast a caught error. Order:networkError→graphQLErrors→bodyText→message→null.src/commands/export-sql.ts): usesparseApiErrorso the specific API message is shown and tracked.UserErrorsuggesting the--wpcli-commandoption, avoiding the server-side 413.UserError: config validation failures now throwUserErrorinstead of a genericError.Testing
parseApiErrorunit tests in__tests__/lib/utils.jscoveringbodyTextmessage/errors extraction,networkError,graphQLErrors, non-JSON fallback, plainError, and thenullfallback.npx jest __tests__/lib/utils.js— passing.