feat(mondo-fetch): Added mondo fetch #571
Conversation
MicahRus
commented
Mar 19, 2024
•
MicahRus
commented
- Updated base package manger to pnpm
- Added mondo-fetch
Allowed passing token to individual requests(Added tests too)
MicahRus
force-pushed
the
feat/add-mondo-fetch
branch
from
ef7a85a to
12f75f6
Compare
…mmon-libs into feat/add-mondo-fetch
packages/mondo-fetch/src/index.ts
Outdated
|
|
||
| const authorizationToken = requestOptions?.authorizationToken || this.authorizationToken | ||
| if (authorizationToken) | ||
| Object.assign(headers, {Authorization: `Bearer ${authorizationToken}`}) |
There was a problem hiding this comment.
This forces us to use bearer token all the time, and if we have other types, will not work. Maybe we need to think about haveing this as a union of different type of authorization
There was a problem hiding this comment.
Definitely agree.
I think as we're using bearer token for most situations this is fine. We can always come back and add it when necessary.
Just being conscious that it would be good to get this landed and being used.
Signed-off-by: github-actions <[email protected]>
| * @param body Optional parameter that contains a payload to send with the request @default undefined | ||
| * @param onBehalfOf An optional user identifier that will be provided to each request @default undefined | ||
| */ | ||
| export interface RequestOptions { |
There was a problem hiding this comment.
I think support for headers and query parameters is required for a http client
packages/mondo-fetch/src/index.ts
Outdated
| message: `The request has reached the maximum duration of ${timeout} milliseconds`, | ||
| }) | ||
|
|
||
| const message = 'We were not able to determine the type of error for the failed request' |
There was a problem hiding this comment.
Fetch often throws when you get cors errors and destination not found errors. We could do a bit more checking on the error name to get a nice message
There was a problem hiding this comment.
Also, don't think we should be logging in here. Libraries in my opinion should not log and instead interact purely through values. Logs you can't control or hide as users is annoying.
| export enum ContentTypes { | ||
| XFORM = 'application/x-www-form-urlencoded', | ||
| JSON = 'application/json; charset=utf-8' | ||
| } |
There was a problem hiding this comment.
Content types need to be more flexible. In HTML spec, content type can be any of the iana mime types. Might be better with a string union of common types and string to get intellisense
https://www.iana.org/assignments/media-types/media-types.xhtml
Changed the implementation of timeouts to use AbortSignal.timeout instead of setTimeouts and AbortControllers. added tests for getUrl
Signed-off-by: github-actions <[email protected]>
