Deprecate request/requestretry – Switch to Axios/axios-retry, Add Central getStream Helper#656
Deprecate request/requestretry – Switch to Axios/axios-retry, Add Central getStream Helper#656yashkohli88 wants to merge 14 commits intoclearlydefined:masterfrom
Conversation
2df3a05 to
fc6d7b5
Compare
pombredanne
left a comment
There was a problem hiding this comment.
Thanks. This looks great to me. Could you try to use an explicit value for the exponential delay?
248ed43 to
77b8c55
Compare
342ee63 to
ab1607d
Compare
| }) | ||
| } | ||
|
|
||
| options.resolveWithFullResponse ??= true |
There was a problem hiding this comment.
This mutates the caller's options object. I'd maybe use a local variable here instead?
| options.resolveWithFullResponse ??= true | |
| const resolveWithFullResponse = options.resolveWithFullResponse ?? true |
| if (!options.resolveWithFullResponse) return response | ||
| return { | ||
| statusCode: response.status, | ||
| headers: response.headers, | ||
| body: response.data, | ||
| request: response.request | ||
| } | ||
| } catch (err) { | ||
| if (err.response) { | ||
| return { | ||
| statusCode: err.response.status, | ||
| headers: err.response.headers, | ||
| body: err.response.data, | ||
| request: err.response.request | ||
| } | ||
| } | ||
| throw err | ||
| } | ||
| } |
There was a problem hiding this comment.
On success with resolveWithFullResponse: false, callers get raw data (e.g. response.crates). On error, they get { statusCode, headers, body, request } regardless. Callers in top.js that access response.packages or response.crates will get undefined on a 5xx instead of a thrown error. The catch block needs to respect this flag.
|
|
||
| if (request.gzip) { | ||
| if (!request.headers) request.headers = {} | ||
| request.headers['Accept-Encoding'] = 'gzip' |
There was a problem hiding this comment.
nit: Axios already sends Accept-Encoding: gzip, deflate, br by default. Explicitly setting gzip overwrites that and drops deflate/br.
Overview
This PR migrates away from the deprecated
requestandrequestretrylibraries in favor of an axios-based solution (axios,axios-retry). A new helper,getStream, is introduced to handle streaming downloads across package fetchers, reducing maintenance burden and improving long-term support.Main Changes
Replaces usage of
requestin all package fetchers (conda, debian, go, maven, npm, composer, pypi, ruby) with a newgetStreamhelper inlib/fetch.js.Replaces
requestretrywithaxios-retryfor HTTP retry support. Centralizes retry logic incallFetchWithRetry.Removes now-unnecessary callback-based code and error handling, moving to promise-based, modern patterns.
Updates unit tests and stubs to use new axios-based methods and async/await.
All fetchers now consistently return streams and/or promise-based results.
Adds exponential delay to retries.
Notable File/Code Changes
getStreamandcallFetchWithRetry.axios+axios-retry.request/requestretryto the new helpers.requestandrequestretrydependencies.Backwards Compatibility