Skip to content

Deprecate request/requestretry – Switch to Axios/axios-retry, Add Central getStream Helper#656

Open
yashkohli88 wants to merge 14 commits intoclearlydefined:masterfrom
yashkohli88:yk/update-requestretry
Open

Deprecate request/requestretry – Switch to Axios/axios-retry, Add Central getStream Helper#656
yashkohli88 wants to merge 14 commits intoclearlydefined:masterfrom
yashkohli88:yk/update-requestretry

Conversation

@yashkohli88
Copy link
Contributor

Overview

This PR migrates away from the deprecated request and requestretry libraries 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

  • Refactor Download Logic:
    Replaces usage of request in all package fetchers (conda, debian, go, maven, npm, composer, pypi, ruby) with a new getStream helper in lib/fetch.js.
  • Retry Logic Update:
    Replaces requestretry with axios-retry for HTTP retry support. Centralizes retry logic in callFetchWithRetry.
  • Code Simplification & Maintenance:
    Removes now-unnecessary callback-based code and error handling, moving to promise-based, modern patterns.
  • Test Enhancements:
    Updates unit tests and stubs to use new axios-based methods and async/await.
  • API Consistency:
    All fetchers now consistently return streams and/or promise-based results.
  • Exponential Backoff Support:
    Adds exponential delay to retries.

Notable File/Code Changes

  • lib/fetch.js
    • Adds getStream and callFetchWithRetry.
    • Switches HTTP requests and retry to axios + axios-retry.
  • *All providers/fetch/Fetch.js
    • Refactored from request/requestretry to the new helpers.
    • Error handling and stream logic updated to promises and async/await.
  • package.json
    • Removes request and requestretry dependencies.
  • Tests
    • Refactored all related test stubs and mocks to support new interfaces.

Backwards Compatibility

  • Code maintains the same external behavior for package downloads and retries; only the underlying HTTP/request implementation has changed.

@yashkohli88 yashkohli88 force-pushed the yk/update-requestretry branch from 2df3a05 to fc6d7b5 Compare February 16, 2026 16:11
@yashkohli88 yashkohli88 marked this pull request as ready for review February 16, 2026 16:16
Copy link
Member

@pombredanne pombredanne left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks. This looks great to me. Could you try to use an explicit value for the exponential delay?

})
}

options.resolveWithFullResponse ??= true
Copy link
Contributor

@JamieMagee JamieMagee Feb 23, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This mutates the caller's options object. I'd maybe use a local variable here instead?

Suggested change
options.resolveWithFullResponse ??= true
const resolveWithFullResponse = options.resolveWithFullResponse ?? true

Comment on lines +100 to +118
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
}
}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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'
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: Axios already sends Accept-Encoding: gzip, deflate, br by default. Explicitly setting gzip overwrites that and drops deflate/br.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants