chore: standardize error responses and add duplicate deck warning#82
chore: standardize error responses and add duplicate deck warning#82TytaniumDev merged 1 commit intomainfrom
Conversation
…#5, #14) - Add `ApiErrorResponse` and `ApiUpdateResponse` shared types in shared/types/api.ts - Create api/lib/api-response.ts with errorResponse, notFoundResponse, badRequestResponse helpers - Replace all ad-hoc NextResponse.json({ error }) patterns across 28 API route files with standard helper functions for consistent error shape - Remove unused REQUIRED_DECK_COUNT re-export from api/lib/types.ts - Add duplicate deck detection warning in POST /api/jobs response Intentionally left unchanged: - Sim PATCH idempotent { updated: false, reason } responses (200s for Pub/Sub) - Recover route { status: 'ok'/'terminal'/'not_found' } success responses - Access request approve route (returns HTML) - Auth helpers unauthorizedResponse/forbiddenResponse (use new Response with custom headers) Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Summary of ChangesHello, 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 robustness and user experience by standardizing error responses and introducing clearer type definitions for API interactions. It centralizes error handling logic, making the API more predictable and easier to consume, while also providing a helpful warning for users submitting jobs with potentially problematic duplicate decks. Highlights
Changelog
Activity
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 a significant improvement by standardizing API error responses. The new helper functions (errorResponse, notFoundResponse, badRequestResponse) and shared types make the error handling more consistent, maintainable, and robust across the application. The addition of a warning for duplicate decks is also a valuable enhancement for users.
The refactoring has been applied systematically across numerous files, and the implementation is clean. The suggestion to further centralize error response generation by creating a helper for unauthorized responses is a valuable next step to complete the standardization effort, and should be addressed in this PR. Overall, this is a high-quality contribution.
| export async function POST(req: NextRequest) { | ||
| if (!IS_LOCAL_MODE && !isWorkerRequest(req)) { | ||
| return NextResponse.json({ error: 'Unauthorized' }, { status: 401 }); | ||
| return errorResponse('Unauthorized', 401); |
There was a problem hiding this comment.
While this correctly uses the new errorResponse helper, to further improve standardization, you could abstract this into a dedicated unauthorizedResponse helper within api-response.ts, similar to notFoundResponse and badRequestResponse.
This would centralize all common HTTP error responses. You could define it in api-response.ts as:
export function unauthorizedResponse(message: string = 'Unauthorized'): NextResponse<ApiErrorResponse> {
return errorResponse(message, 401);
}This would also allow for refactoring the existing unauthorizedResponse in lib/auth.ts to use this new helper, making all error responses flow through api-response.ts for maximum consistency.
References
- Address valid code quality suggestions immediately within the current pull request, even if it expands the scope. Do not defer them to a follow-up task.
1 participant
Summary
ApiErrorResponseandApiUpdateResponseshared types inshared/types/api.tsapi/lib/api-response.tswitherrorResponse,notFoundResponse,badRequestResponsehelpersREQUIRED_DECK_COUNTre-export fromapi/lib/types.tsAddresses architecture issues #4, #5, and #14 from
docs/ARCHITECTURE_ISSUES.md.Test plan
{ error: string }shape{ updated: false, reason })npm run lint --prefix apipassesnpm run build --prefix apipassesnpm run lint --prefix frontendpassesnpm run build --prefix frontendpasses🤖 Generated with Claude Code