-
Notifications
You must be signed in to change notification settings - Fork 99
Refactor API error handling for consistency and maintainability #104
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Refactor API error handling for consistency and maintainability #104
Conversation
Standardize error responses across the AIAssistant API endpoint by utilizing centralized error handling utilities from api-utils module. Changes: **Added to constants.ts:** - HTTP_STATUS.UNAUTHORIZED (401) - HTTP_STATUS.FORBIDDEN (403) **Added to api-utils.ts:** - createUnauthorizedError() - handles 401 unauthorized responses - createForbiddenError() - handles 403 forbidden responses **Refactored AIAssistant/route.ts:** - Replace manual NextResponse.json() error construction with standardized error handling functions - Import and use: createUnauthorizedError, createForbiddenError, createNotFoundError, createValidationError, handleApiError - All error responses now follow consistent structure with proper error types, timestamps, and HTTP status codes Benefits: - Improved code maintainability through centralized error handling - Consistent error response format across all endpoints - Better type safety and reduced code duplication - Easier debugging with standardized error types - Development mode error details support built-in This refactoring maintains backward compatibility while improving the internal code quality and setting a pattern for other API endpoints to follow.
|
@friendlybureau is attempting to deploy a commit to the Timothy Lin's projects Team on Vercel. A member of the Team first needs to authorize it. |
| success: false, | ||
| message: "No relevant content found for the given question and document.", | ||
| }); | ||
| return createNotFoundError("No relevant content found for the given question and document"); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
| return createNotFoundError("No relevant content found for the given question and document"); | |
| return NextResponse.json({ | |
| success: false, | |
| message: "No relevant content found for the given question and document." | |
| }); |
The "No relevant content found" response returns HTTP 404, but this represents a breaking change from the original behavior (200 status). A 404 status code is semantically incorrect here since it indicates a resource doesn't exist, not that a search query returned no results.
View Details
Analysis
AI Assistant API returns HTTP 404 instead of 200 for empty search results
What fails: AIAssistant POST endpoint at /api/AIAssistant/route.ts:312 returns HTTP 404 when no documents match a search query, violating HTTP semantics for empty search results
How to reproduce:
# POST to /api/AIAssistant with valid query that matches no documents
curl -X POST /api/AIAssistant -H "Content-Type: application/json" -d '{"documentId": 999, "question": "test"}'Result: Returns HTTP 404 with error response. Commit 7feffbe changed from NextResponse.json() (defaulting to 200) to createNotFoundError() (returning 404)
Expected: Should return HTTP 200 with success response containing empty results, per HTTP semantics and API best practices - empty search results are valid responses, not missing resources
| export function createUnauthorizedError( | ||
| message = "Unauthorized", | ||
| error?: Error | string | ||
| ): NextResponse<ErrorResponse> { | ||
| return createErrorResponse( | ||
| message, | ||
| ERROR_TYPES.VALIDATION, | ||
| HTTP_STATUS.UNAUTHORIZED, | ||
| error | ||
| ); | ||
| } | ||
|
|
||
| /** | ||
| * Create a forbidden error response | ||
| */ | ||
| export function createForbiddenError( | ||
| message = "Access forbidden", | ||
| error?: Error | string | ||
| ): NextResponse<ErrorResponse> { | ||
| return createErrorResponse( | ||
| message, | ||
| ERROR_TYPES.VALIDATION, | ||
| HTTP_STATUS.FORBIDDEN, | ||
| error | ||
| ); | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Both createUnauthorizedError (line 102) and createForbiddenError (line 117) use ERROR_TYPES.VALIDATION, which is semantically incorrect for authentication/authorization errors.
View Details
📝 Patch Details
diff --git a/src/lib/api-utils.ts b/src/lib/api-utils.ts
index 24bde9d..0650dc5 100644
--- a/src/lib/api-utils.ts
+++ b/src/lib/api-utils.ts
@@ -99,7 +99,7 @@ export function createUnauthorizedError(
): NextResponse<ErrorResponse> {
return createErrorResponse(
message,
- ERROR_TYPES.VALIDATION,
+ ERROR_TYPES.AUTHENTICATION,
HTTP_STATUS.UNAUTHORIZED,
error
);
@@ -114,7 +114,7 @@ export function createForbiddenError(
): NextResponse<ErrorResponse> {
return createErrorResponse(
message,
- ERROR_TYPES.VALIDATION,
+ ERROR_TYPES.AUTHORIZATION,
HTTP_STATUS.FORBIDDEN,
error
);
@@ -232,4 +232,4 @@ export function validateRequest<T>(
error: error instanceof Error ? error.message : "Validation failed"
};
}
-}
\ No newline at end of file
+}
diff --git a/src/lib/constants.ts b/src/lib/constants.ts
index b1811ad..10cb27f 100644
--- a/src/lib/constants.ts
+++ b/src/lib/constants.ts
@@ -63,6 +63,8 @@ export type UrgencyLevel = typeof URGENCY_LEVELS[number];
// Error Types
export const ERROR_TYPES = {
VALIDATION: 'validation',
+ AUTHENTICATION: 'authentication',
+ AUTHORIZATION: 'authorization',
TIMEOUT: 'timeout',
DATABASE: 'database',
EXTERNAL_SERVICE: 'external_service',
Analysis
Incorrect error type classification for authentication and authorization errors
What fails: Functions createUnauthorizedError() and createForbiddenError() in src/lib/api-utils.ts both return errorType: "validation" in their JSON responses, which semantically misclassifies authentication (401) and authorization (403) errors as validation errors.
How to reproduce:
// Test the error types returned
import { createUnauthorizedError, createForbiddenError } from './src/lib/api-utils';
const auth = createUnauthorizedError("test");
const authz = createForbiddenError("test");
console.log(await auth.json()); // Shows: "errorType": "validation"
console.log(await authz.json()); // Shows: "errorType": "validation"Result: Both functions return errorType: "validation" despite handling authentication/authorization failures, not input validation failures.
Expected: Authentication errors (401) should have errorType: "authentication" and authorization errors (403) should have errorType: "authorization" per REST API error handling best practices which distinguish authentication/authorization from validation errors.
Impact: Affects error categorization in logs, monitoring systems, and any client code that conditions behavior on errorType. While not currently breaking functionality, it degrades semantic accuracy of API responses.
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
Summary
This PR refactors the AIAssistant API endpoint to use standardized error handling utilities, improving code consistency and maintainability across the project.
Problem
Currently, error responses in the AIAssistant API are manually constructed using
NextResponse.json(), leading to:Changes
1. Enhanced Constants (
src/lib/constants.ts)HTTP_STATUS.UNAUTHORIZED(401)HTTP_STATUS.FORBIDDEN(403)2. New Error Utilities (
src/lib/api-utils.ts)createUnauthorizedError()- Handles 401 unauthorized responsescreateForbiddenError()- Handles 403 forbidden access responses3. Refactored AIAssistant API (
src/app/api/AIAssistant/route.ts)Replaced all manual error construction with standardized functions:
Before:
After:
Errors Standardized:
Benefits
✨ Improved Maintainability
📊 Consistent API Responses
🛡️ Better Type Safety
🐛 Enhanced Debugging
♻️ Reusability
Testing
Code Quality Metrics
Response Format Example
{ "success": false, "message": "Unauthorized", "errorType": "validation", "timestamp": "2025-11-14T01:45:00.000Z", "error": "Additional details (dev mode only)" }Related Files
src/lib/constants.ts- Added HTTP status code constantssrc/lib/api-utils.ts- Added error utility functionssrc/app/api/AIAssistant/route.ts- Refactored to use utilitiesFuture Improvements
This refactoring sets the foundation for:
Note: This is a non-breaking refactoring that maintains full backward compatibility with existing clients while improving internal code quality.