fix(request): Fix request() w/ relevant docs #36
Conversation
src/request.ts
Outdated
| @@ -5,19 +5,37 @@ import 'isomorphic-fetch'; | |||
| * Return a promise that is resolved or rejected depending on the response's | |||
| * status code. | |||
| */ | |||
| export const checkStatus = (res: Response): Promise<Response> => { | |||
| const _checkStatus = (res: Response): Promise<Response> => { | |||
| if (res.status >= 300) { | |||
There was a problem hiding this comment.
why 300s? with our api implementation this shouldnt be an issue, but why not reject >= 400's
There was a problem hiding this comment.
That's a good call. It's what the code internally was doing, but I agree it should be >= 400. A 301 redirect for instance shouldn't trigger this
| new Promise((resolve, reject) => { | ||
| response | ||
| .json() |
There was a problem hiding this comment.
was the issue that the actual parsing was failing and overriding the potential error from the api?
There was a problem hiding this comment.
Yeah if for whatever reason we weren't getting back valid JSON, this was failing. It was one of the new test cases I added. Our actual code was failing instead of us providing a rejected promise
|
|
||
| See the [Error Handling](#error-handling) section for more information on the default error handling that `request()` provides. | ||
|
|
||
| ## Examples |
There was a problem hiding this comment.
What do you think about giving a quick shoutout to the fact that you can use this sdk on the client or on the server? Not sure where it would live.. is it just assumed that libraries are using isomorphic fetch these days?
There was a problem hiding this comment.
I think I'll do this in #33 where the main README is getting updated. Think this is a good idea
docs/README.md
Outdated
|
|
||
| ## Configuring a SDK object | ||
|
|
||
| In order to make requests, we need to configure the SDK object. |
There was a problem hiding this comment.
nit: might change the 'we' to 'you' since you use 'you' in later sentences
|
🎉 This PR is included in version 1.0.0 🎉 The release is available on: Your semantic-release bot 📦🚀 |
Description
The intention of this PR was just to write the docs for the
request()method. But while writing the docs, I realized that the error handling was broken in certain cases. So that got fixed along with the tests.So what ended up happening:
request()methodrequest()the only export inrequest.tscrequest.tscrequest()and to test for more error casesPULL_REQUEST_TEMPLATE.md&CONTRIBUTING.mdfrom chore(build): Add semantic-release in Travis #33How Has This Been Tested?
Updated and additional unit tests
Checklist