-
Notifications
You must be signed in to change notification settings - Fork 34
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
[93] Updated response formatting interceptor #142
Conversation
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the You can disable this status message by setting the WalkthroughRecent updates enhance file upload handling and response formatting functionalities. The Changes
Poem
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (invoked as PR comments)
Additionally, you can add CodeRabbit Configration File (
|
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.
Actionable comments posted: 1
Outside diff range and nitpick comments (1)
packages/common/src/interceptors/file-upload.interceptor.ts (1)
Line range hint
23-24
: Decorator usage issue flagged by static analysis.The static analysis tool indicates an improper usage of decorators. In TypeScript, decorators can only be used on class declarations, class expressions, and class methods. To solve this, ensure that decorators are used correctly or adjust the TypeScript configuration if necessary.
- @Optional() - @Inject('MULTER_MODULE_OPTIONS') + // Decorators removed due to incorrect usage. Consider appropriate alternatives or enable unsafe decorators in TS config.
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files ignored due to path filters (1)
packages/user-service/yarn.lock
is excluded by!**/yarn.lock
,!**/*.lock
Files selected for processing (4)
- packages/common/src/interceptors/file-upload.interceptor.ts (1 hunks)
- packages/common/src/interceptors/response-format.interceptor.ts (1 hunks)
- packages/common/test/file-upload.interceptor.spec.ts (1 hunks)
- sample/03-response-formatting/src/main.ts (1 hunks)
Additional context used
Biome
packages/common/src/interceptors/file-upload.interceptor.ts
[error] 23-24: Decorators are not valid here.
Decorators are only valid on class declarations, class expressions, and class methods.
You can enable parameter decorators by setting theunsafeParameterDecoratorsEnabled
option totrue
in your configuration file.(parse)
packages/common/src/interceptors/response-format.interceptor.ts
[error] 41-41: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
Additional comments not posted (3)
sample/03-response-formatting/src/main.ts (1)
3-3
: Updated import path forResponseFormatInterceptor
.The import path update to use the package '@samagra-x/stencil' instead of a local path helps in maintaining modularity and ensures that the interceptor is managed as part of a package, which is good for maintainability.
packages/common/src/interceptors/response-format.interceptor.ts (1)
17-35
: Enhanced response structure in interceptor.The updates to include a
success
flag, and more detailedmessage
,data
, anderror
fields enhance the clarity and utility of the response structure. These changes are crucial for consistent error handling and response formatting.packages/common/test/file-upload.interceptor.spec.ts (1)
5-62
: Comprehensive test suite forFastifyFileInterceptor
.The tests cover various scenarios including normal operations, error handling, and edge cases like missing files. This is good practice to ensure that the interceptor behaves as expected under different conditions.
private formatErrorMessage(error: any): string { | ||
if (error.response && error.response.message) { | ||
return error.response.message; | ||
} | ||
return error.message || 'Internal Server Error'; | ||
} | ||
|
||
private formatError(error: any): any { | ||
// Format your error object here. This can be customized as per requirements. | ||
if (error.response) { | ||
return { | ||
code: error.response.statusCode, | ||
message: error.response.message, | ||
statusCode: | ||
error.response.statusCode || HttpStatus.INTERNAL_SERVER_ERROR, | ||
}; | ||
} | ||
return { | ||
code: error.status || HttpStatus.INTERNAL_SERVER_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.
Refactor error handling methods to use optional chaining.
The static analysis tool suggests using optional chaining to simplify the code. This will make the code cleaner and reduce the possibility of runtime errors.
- if (error.response && error.response.message) {
+ return error.response?.message || 'Internal Server Error';
- }
- return error.message || 'Internal Server Error';
Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
private formatErrorMessage(error: any): string { | |
if (error.response && error.response.message) { | |
return error.response.message; | |
} | |
return error.message || 'Internal Server Error'; | |
} | |
private formatError(error: any): any { | |
// Format your error object here. This can be customized as per requirements. | |
if (error.response) { | |
return { | |
code: error.response.statusCode, | |
message: error.response.message, | |
statusCode: | |
error.response.statusCode || HttpStatus.INTERNAL_SERVER_ERROR, | |
}; | |
} | |
return { | |
code: error.status || HttpStatus.INTERNAL_SERVER_ERROR, | |
private formatErrorMessage(error: any): string { | |
return error.response?.message || error.message || 'Internal Server Error'; | |
} | |
private formatError(error: any): any { | |
if (error.response) { | |
return { | |
code: error.response.statusCode, | |
message: error.response.message, | |
}; | |
} | |
return { | |
code: error.status || HttpStatus.INTERNAL_SERVER_ERROR, |
Tools
Biome
[error] 41-41: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
packages/common/src/interceptors/response-format.interceptor.ts
Outdated
Show resolved
Hide resolved
packages/common/src/interceptors/response-format.interceptor.ts
Outdated
Show resolved
Hide resolved
packages/common/src/interceptors/response-format.interceptor.ts
Outdated
Show resolved
Hide resolved
packages/common/src/interceptors/response-format.interceptor.ts
Outdated
Show resolved
Hide resolved
Parent issue: #93 |
Updated response formatting interceptor
fixes #141
Demo
Summary by CodeRabbit
New Features
success
flag,message
,data
, anderror
fields for better clarity.Bug Fixes
Tests