Return SwapperError object #54
Conversation
Introduces the SwapperError type to the API and types package, enabling structured error handling for swapper endpoints. API responses for quote and quote_data now use a ProxyResponse type with standardized error objects. The typeshare preparation script now generates SwapperError from the Rust crate to avoid duplication. Minor updates to ProxyQuoteRequest and index exports for consistency.
|
Review changes with Changed Files
|
Summary of ChangesHello @0xh3rman, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request significantly enhances the API's error reporting by implementing a structured Highlights
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request introduces structured error handling by returning SwapperError objects, which is a great improvement for API clients. My review focuses on making this new implementation more robust and maintainable. I've identified a couple of high-severity issues: one related to potential information loss in the new structured error responses, and another concerning unsafe handling of request bodies which could lead to runtime errors. I've also included a medium-severity suggestion to refactor duplicated code into a middleware to improve maintainability. Addressing these points will make the API more reliable and easier to maintain.
apps/api/src/index.ts
Outdated
| const providerId = req.params.providerId; | ||
| const provider = providers[providerId]; | ||
| const version = parseVersion(req.query.v); | ||
| const errorObject = version >= API_VERSION; | ||
|
|
||
| if (!provider) { | ||
| res.status(404).json(errorResponse({ type: "no_available_provider" }, `Provider ${providerId} not found`, errorObject)); | ||
| return; | ||
| } |
There was a problem hiding this comment.
There's significant code duplication between this route handler (/:providerId/quote) and the /:providerId/quote_data handler (lines 68-76). The logic for parsing providerId, looking up the provider, parsing the version, and handling the 'provider not found' case is identical.
To improve maintainability and reduce redundancy, consider extracting this common logic into an Express middleware. The middleware could attach the provider and errorObject flag to the request object for subsequent handlers to use.
Example of what a middleware could look like:
function providerMiddleware(req, res, next) {
const providerId = req.params.providerId;
const provider = providers[providerId];
const version = parseVersion(req.query.v);
// Extend Express.Request type to add these properties
req.provider = provider;
req.errorObject = version >= API_VERSION;
if (!provider) {
res.status(404).json(errorResponse({ type: "no_available_provider" }, `Provider ${providerId} not found`, req.errorObject));
return;
}
next();
}
// Usage:
app.post("/:providerId/quote", providerMiddleware, async (req, res) => {
// req.provider is available here
// ...
});Replaces 'input_amount_too_small' with 'input_amount_error' in SwapperError type and adds a 'min_amount' field to provide more detailed error information.
No description provided.