-
Notifications
You must be signed in to change notification settings - Fork 26
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
Improve errors alternative implementation #212
Conversation
...of #211 where `ApiError` has all the API response properties directly on it instaed of inside a `response` object - `ApiError.response.error` -> `ApiError.code` - `ApiError.response.message` -> `ApiError.rawMessage` - `ApiError.response.assembly_id` -> `ApiError.assemblyId` - `ApiError.response.assembly_ssl_url` -> `ApiError.assemblySslUrl`
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## improve-errors2 #212 +/- ##
==================================================
Coverage ? 68.36%
==================================================
Files ? 6
Lines ? 588
Branches ? 113
==================================================
Hits ? 402
Misses ? 186
Partials ? 0
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
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.
Yes, I really like this! I like it better than #211
Co-authored-by: Remco Haszing <[email protected]>
* Improve errors Renamed `TransloaditError` to `ApiError`. Differences between `TransloaditError` and `ApiError`: - Moved `TransloaditError.response.body` to `ApiError.response` - Removed `TransloaditError.assemblyId` (can now be found in `ApiError.response.assembly_id` - Removed `TransloaditError.transloaditErrorCode` (can now be found in `ApiError.response.error` - `ApiError` does not inherit from `got.HTTPError`, but `ApiError.cause` will be the `got.HTTPError` instance that caused this error (except for when Tranloadit API responds with HTTP 200 and `error` prop set in JSON response, in which case cause will be `undefined`). Note that (just like before) when the Transloadit API responds with an error we will always throw a `ApiError` - In all other cases (like request timeout, connection error, TypeError etc.), we don't wrap the error in `ApiError`. Also improved error stack traces, added a unit test in `mock-http.test.ts` that verifies the stack trace. * Improve errors alternative implementation (#212) * make alternative implementation ...of #211 where `ApiError` has all the API response properties directly on it instaed of inside a `response` object - `ApiError.response.error` -> `ApiError.code` - `ApiError.response.message` -> `ApiError.rawMessage` - `ApiError.response.assembly_id` -> `ApiError.assemblyId` - `ApiError.response.assembly_ssl_url` -> `ApiError.assemblySslUrl` * fix formatting * Update README.md Co-authored-by: Remco Haszing <[email protected]> * remove stack hack #211 (comment) * improve assertion * fix typo * fix formatting --------- Co-authored-by: Remco Haszing <[email protected]> --------- Co-authored-by: Remco Haszing <[email protected]>
Alternative implementation of #211 where
ApiError
has all the API response properties directly on itinstaed of inside a
response
propertyApiError.response.error
->ApiError.code
ApiError.response.message
->ApiError.rawMessage
ApiError.response.assembly_id
->ApiError.assemblyId
ApiError.response.assembly_ssl_url
->ApiError.assemblySslUrl
I kind of like this better (less typing)
closes #154