-
Notifications
You must be signed in to change notification settings - Fork 44
Add ParseError to include rawBody and requestID for debugging purposes #1313
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
Conversation
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.
Greptile Summary
This PR enhances error handling in the WorkOS Node.js SDK by introducing a new ParseError class to handle cases where non-JSON responses are received from the WorkOS API. The changes add important debugging information (raw response body and request ID) when JSON parsing fails.
The implementation involves:
- Adding a new
ParseErrorclass that extendsErrorand implementsRequestException - Modifying the HTTP methods (post, get, put) to capture and include debugging information when parsing fails
- Adding a new
handleParseErrormethod to extract the raw response and request ID
This change will significantly improve debugging capabilities when investigating API response issues, particularly beneficial for support scenarios.
Confidence score: 5/5
- This PR is very safe to merge as it only adds error handling improvements
- The changes are well-contained, only affecting error handling paths, and can't impact successful API calls
- No files need special attention as the changes are straightforward and well-implemented
2 files reviewed, 2 comments
Edit PR Review Bot Settings | Greptile
| readonly name = 'ParseError'; | ||
| readonly status = 500; |
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.
logic: Ensure 500 is the correct status code for parse errors. Consider if 400 might be more appropriate since it's a client-side parsing issue.
Co-authored-by: greptile-apps[bot] <165735046+greptile-apps[bot]@users.noreply.github.com>
mattgd
left a comment
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.
Thanks for adding this. Could we add a test to exercise the ParseError behavior?
mattgd
left a comment
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.
Thanks for implementing this!
## Description Missed a codepath in #1313 where non-200 responses would attempt to parse the body
Description
Some customers have reported receiving non-json responses from the workos API. These raise a syntax error when the Node.js ADK attempts to parse the json. This PR adds rawBody and requestID to a new ParseError exception for debugging purposes.