-
-
Notifications
You must be signed in to change notification settings - Fork 44
feat: implement uniform error handling in lib scripts
#179
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
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
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.
Pull Request Overview
This PR standardizes error handling across the lib scripts by implementing consistent error propagation patterns and improving API error responses. The changes replace generic console logging and error throwing with structured Error objects that include the original error as the cause.
- Replaced direct error throwing with structured Error objects containing descriptive messages and original errors as the cause
- Added try/catch blocks around previously unguarded API calls to ensure proper error handling
- Updated API error response handling to use the new error structure with
error.causeproperties
Reviewed Changes
Copilot reviewed 6 out of 6 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| src/lib/branch.js | Added try/catch blocks and structured error handling for branch operations |
| src/lib/fork.js | Replaced console logging with proper error throwing using structured Error objects |
| src/lib/submit-word.js | Added try/catch wrapper around pull request creation |
| src/lib/word-editor.js | Updated error handling to use structured Error objects with original error as cause |
| src/pages/api/dictionary.js | Updated error responses to use new error structure and fixed cookie key name |
| src/components/islands/word-editor.jsx | Changed class to className attributes and improved error status checking |
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
…n forkRepository function (backlog from #179)
### Description <!-- Please add PR description (don't leave blank) - example: This PR [adds/removes/fixes/replaces] the [feature/bug/etc] --> This pull request introduces a comprehensive set of improvements to the testing infrastructure, primarily focused on mocking GitHub API interactions for more robust and reliable tests. The changes include the addition of the `msw` library for API mocking, new fixture data for various GitHub responses and business logic scenarios, and the setup of a mock server for tests. Additionally, some minor code cleanups were made in `src/lib/fork.js` for error handling and logging. ### Key Changes * Added the `msw` library to `package.json` to enable HTTP and GraphQL API mocking in tests. * Created new fixtures in `tests/fixtures/github-responses/index.js` and `tests/fixtures/test-data/index.js` to provide realistic mock data for GitHub API responses and business logic scenarios. [[1]](diffhunk://#diff-cbdac2b85312f0dad66d839fab58659e52d8f132934955f48f437bdd08ef0a0fR1-R188) [[2]](diffhunk://#diff-ac3a67c5bc05dc776d1de526fb36b63154259d4342e804067f78734da315ddebR1-R112) * Implemented `tests/mocks/github-api.js` with a comprehensive set of MSW handlers to mock GitHub REST and GraphQL endpoints, including user info, PR creation, branch management, file content operations, forking, and error simulation. * Set up the MSW server in `tests/setup.js` to automatically start, reset, and stop the mock server during test lifecycle events, ensuring consistent and isolated API mocking for all tests. ### Unrelated Changes * Improved error handling and cleaned up console logging in `src/lib/fork.js` by commenting out debug logs and updating error throwing to use the `cause` property, providing clearer error context for fork synchronization failures. Backlog from #179 ### Related Issue <!-- Please prefix the issue number with Fixes/Resolves - example: Fixes #123 or Resolves #123 --> Fixes #172 ### Screenshots/Screencasts <!-- Please provide screenshots or video recording that demos your changes (especially if it's a visual change) --> [Video Screen1757194048382.webm](https://github.com/user-attachments/assets/f1ffce2d-0137-4987-ba94-c2d59ec86822) ### Notes to Reviewer <!-- Please state here if you added a new npm packages, or any extra information that can help reviewer better review you changes --> - Added `msw` npm package to dev dependencies
Description
This pull request focuses on improving error handling and consistency across
libscripts, standardizing error propagation, enhancing API error responses. Also made minor UI fixes in the word editor component to adapt to new error structure.Notable Changes
Replaced generic error throwing and console logging with explicit
Errorobjects that include the original error as thecauseinsrc/lib/branch.js,src/lib/fork.js,src/lib/submit-word.js, andsrc/lib/word-editor.js. This standardizes error propagation and makes debugging easier. [1] [2] [3] [4] [5] [6] [7] [8] [9]Wrapped API calls in
try/catchblocks to ensure errors are properly caught and propagated, improving reliability and making error handling more consistent. [1] [2] [3]Updated error responses in
src/pages/api/dictionary.jsto useerror.cause.messageanderror.cause.statusfor more accurate client feedback, and fixed the cookie key tojargonsdevTokenfor consistency. [1] [2] [3]Other Unrelated Changes
classattributes toclassNamein the React componentTogglePreviewand its children, aligning with React conventions and preventing potential rendering issues.Editorcomponent to use HTTP status 422 and substring matching for error messages, making validation more robust.Related Issue
Fixes #16
Screenshots/Screencasts
NA
Notes to Reviewer
NA