-
Notifications
You must be signed in to change notification settings - Fork 2.4k
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
Handle query runner errors #6424
Conversation
thomtrp
commented
Jul 26, 2024
- Throw service error from query runner
- Catch in resolver factories
- Map to graphql errors
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.
PR Summary
This pull request introduces comprehensive error handling improvements for the query runner, ensuring service-specific exceptions are mapped to GraphQL errors.
- Enhanced Error Handling: Replaced generic exceptions with
WorkspaceQueryRunnerException
across multiple files for more granular error control. - New Utility Function: Added
workspace-query-runner-graphql-api-exception-handler.util.ts
to map custom exceptions to GraphQL errors. - Custom Exception Class: Introduced
workspace-query-runner.exception.ts
to standardize error handling within the workspace query runner. - Resolver Updates: Modified resolver factories to catch and handle errors using the new exception handler utility.
- GraphQL Config Update: Replaced
useExceptionHandler
withuseGraphQLErrorHandlerHook
ingraphql-config.service.ts
for improved error handling.
16 file(s) reviewed, 1 comment(s)
Edit PR Review Bot Settings
@@ -49,7 +58,7 @@ export const computePgGraphQLError = ( | |||
const errorMessage = error?.message; | |||
|
|||
const mappedErrorKey = Object.keys(pgGraphQLErrorMapping).find( | |||
(key) => errorMessage?.startsWith(key), | |||
(key) => errorMessage?.contains(key), |
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.
Style: Use of contains instead of startsWith for error message matching.
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.
LGTM, I've remove the unused hook and replaced string.contains by string.includes that seems more legit