Skip to content

Comments

Swap route error message fix#531

Open
neavra wants to merge 3 commits intomainfrom
swap-route-fix-error-message
Open

Swap route error message fix#531
neavra wants to merge 3 commits intomainfrom
swap-route-fix-error-message

Conversation

@neavra
Copy link
Contributor

@neavra neavra commented Feb 2, 2026

Summary by CodeRabbit

  • Bug Fixes
    • Improved handling of plugin validation failures: validation errors now return a clear, user-facing message and a 400 response so users get actionable feedback.
    • Other plugin errors continue to surface as before (no change to non-validation error behavior).

@coderabbitai
Copy link
Contributor

coderabbitai bot commented Feb 2, 2026

Walkthrough

Adds a ValidationError type for user-facing plugin validation messages, returns HTTP 400 from the plugin server when such errors occur, and updates the API layer to detect and extract validation messages from plugin HTTP 400 responses.

Changes

Cohort / File(s) Summary
Plugin Error Type
plugin/errors.go
Adds ValidationError (fields Err error, Message string), implements Error() and Unwrap(), and adds NewValidationError constructor.
Validation handling (server + API)
plugin/server/server.go, internal/api/plugin.go
Server: detect *plugin.ValidationError and respond with HTTP 400 and the validation message. API: import github.com/vultisig/verifier/plugin/libhttp and parse plugin HTTP 400 (libhttp.HTTPError) to extract JSON validation messages for API responses.
🚥 Pre-merge checks | ✅ 2 | ❌ 1
❌ Failed checks (1 warning)
Check name Status Explanation Resolution
Title check ⚠️ Warning The title refers to a 'swap route error message fix' but the changeset adds generalized plugin validation error handling with new error types and updates multiple files beyond swap routes. Update the title to reflect the broader scope, such as 'Add plugin validation error handling' or 'Improve plugin error message handling for HTTP 400 responses'.
✅ Passed checks (2 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Docstring Coverage ✅ Passed Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing touches
  • 📝 Generate docstrings
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch swap-route-fix-error-message

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@neavra neavra marked this pull request as ready for review February 2, 2026 13:35
Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 2

🤖 Fix all issues with AI agents
In `@internal/api/plugin.go`:
- Around line 703-715: The handler treats plugin validation errors as HTTP 500;
change it to return HTTP 400: in internal/api/plugin.go where errors.As(err,
&httpErr) && httpErr.StatusCode == http.StatusBadRequest is checked, replace
c.JSON(http.StatusInternalServerError, ...) with c.JSON(http.StatusBadRequest,
...) for both the case that parses errResp.Message and the fallback that returns
"invalid request", so the function returns a 400 response using
NewErrorResponseWithMessage containing the plugin-provided message or the
fallback text.

In `@plugin/errors.go`:
- Around line 11-21: The ValidationError.Error method (and Unwrap) can panic if
the receiver or its Err is nil; update Error() to defensively handle a nil
receiver and nil e.Err by returning a safe string (e.g., empty string or the
Message) when nil, and update Unwrap() to return nil if receiver is nil; ensure
NewValidationError remains unchanged but mention ValidationError, Error(), and
Unwrap() so you modify those methods to perform nil checks before dereferencing
e.Err.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant