Migrate from request to node-fetch #54
Conversation
|
@avh4 any chance you merge this soon? #65 is a big issue for a couple of important Elm related npm packages that use this package (elm-format and elm-json). Otherwise, here is a list of other possible alternatives request/request#3143 Thanks |
| reject("Error communicating with URL " + url + " " + error); | ||
| return; | ||
| } | ||
| if (response.statusCode == 404) { |
There was a problem hiding this comment.
According to https://www.npmjs.com/package/node-fetch#handling-exceptions
3xx-5xx responses are NOT exceptions, and should be handled in then()
Doesn't that mean we still need to check for 404 or any other 300-599 status codes?
| .pipe(gunzip) | ||
| .pipe(untar); | ||
| return |
There was a problem hiding this comment.
Don't we need to return the stream or the promise or whatever so that the caller will be able to wait for the operation to fully complete? (Or is that happening somehow as this is?)
| .then( | ||
| function () { | ||
| console.log("OKAY: " + displayUrl); | ||
| return |
There was a problem hiding this comment.
We still need to check for statusCode != 200 here.
Hi, this package looks interesting for my purposes, but the request deprecation warning is nasty. Could it be a possible approach to take smaller steps and migrate to a lighter HTTP client? I have migrated from request to node-fetch in my fork. This gets rid of about 40 dependencies, the tests run 2 seconds faster, and node-fetch supports redirects.