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
Update PDF util #4742
Update PDF util #4742
Conversation
server/lib/fetch.js
Outdated
* @param {Number} [timeoutInMs] The callback to call with the authorized client. Default is 5000 ms. | ||
*/ | ||
|
||
export const fetchWithTimeout = (fetchParams, timeoutInMs = 5000) => { |
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.
I think it would look better by following fetch
API: first argument url
, second argument options
. timeoutInMs
could be read from options
or kept as third argument.
f1e5b06
to
496328f
Compare
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.
The helper looks good to me, using async/await may have been cleaner - promises chains are a bit difficult to read. But that's not a blocker.
I'll keep this in mind and maybe refactor next week when I have a bit more time. Was good to practice promises at least 😅 |
496328f
to
297215c
Compare
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.
Looks good. I'm not sure if that's needed but we could also add Keep-Alive.
Similar to https://github.com/opencollective/opencollective-frontend/blob/master/lib/apollo-client.js#L49
297215c
to
eaaa801
Compare
fetchWithTimeout
which takes a URL and options to pass tonode-fetch
and a timeout in ms, and returns a thenable promise