chore(profile): replace request with native fetch - #20995
Open
toufali wants to merge 1 commit into
Open
Conversation
toufali
force-pushed
the
dep-reduce-request-3
branch
from
August 7, 2026 23:04
dbc3d91 to
ce3a739
Compare
toufali
marked this pull request as ready for review
August 7, 2026 23:05
Contributor
There was a problem hiding this comment.
Pull request overview
This PR removes the deprecated request library from fxa-profile-server by migrating the remaining outbound HTTP calls (OAuth verify, auth-server profile lookup, and image-worker requests) to native fetch, and updates tests to mock fetch with Jest instead of nock.
Changes:
- Migrates OAuth
/verify, auth-server/account/profile, and image-worker upload/delete calls fromrequestto nativefetch. - Replaces
nock-based test mocking with aglobal.fetchstub that asserts all expected requests were consumed and can inject requests into the real worker. - Drops
request,nock, andthrough, and deletes the now-unused helper modules (lib/request.js,test/lib/inject.js).
Reviewed changes
Copilot reviewed 8 out of 9 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| yarn.lock | Removes direct dependencies on request, nock, and through from the lockfile graph (where possible). |
| packages/fxa-profile-server/package.json | Drops request, nock, and through dependencies from the profile-server package. |
| packages/fxa-profile-server/lib/server/web.js | Replaces OAuth /verify call with fetch in the auth scheme. |
| packages/fxa-profile-server/lib/routes/_core_profile.js | Replaces auth-server profile lookup with fetch + timeout via AbortSignal.timeout. |
| packages/fxa-profile-server/lib/img-workers.js | Replaces worker upload/delete HTTP calls with streaming fetch (including duplex: 'half'). |
| packages/fxa-profile-server/lib/request.js | Deletes the request wrapper and agent/socket tuning. |
| packages/fxa-profile-server/test/lib/mock.js | Replaces nock request interception with a global.fetch stub + outstanding-call assertions and worker injection. |
| packages/fxa-profile-server/test/lib/inject.js | Deletes the old through-based inject helper used by nock mocks. |
| packages/fxa-profile-server/test/load/avatar/avatar.js | Updates avatar load harness to use native fetch for upload/download/delete. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
toufali
force-pushed
the
dep-reduce-request-3
branch
from
August 7, 2026 23:33
ce3a739 to
dbf8b98
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Because
requestis deprecated upstream (Request’s Past, Present and Future request/request#3142), and this was the last first-party usage in the repo.fetch, and upgrading it was already tried and rejected (task: upgrade jest and nock to support native fetch #18724), so the mocks move to plain Jestfetchstubs — matching the axios conversions.This pull request
/verifycall, the auth-server profile lookup, and both image-worker calls to nativefetch; deleteslib/request.js.test/lib/mock.jswith aglobal.fetchstub, somock.done()still asserts every mock was consumed. The worker mock buffers the body and injects it into the real worker, keeping the image pipeline under test.request,nock, andthrough. Also removes the S3 mock helpers, dead sinceimg.driverislocalin the test config.Issue that this pull request solves
Closes: FXA-4892
Checklist
Put an
xin the boxes that applyHow to review
lib/img-workers.jsis the risky part: the avatar upload streams its body, and the worker route requirescontent-length. Forwarding the caller's header keeps it a fixed-length request — without it undici falls back to chunked encoding./verifybody is now treated as absent, so a bare string can't be returned as credentials; andrequest'sforever/maxSocketstuning is replaced by undici's pooling, sincefetchhas noagentoption. (This worked for our Axios replacement as well.)Other information (Optional)
requeststill resolves in the lockfile vianode-zendesk,grunt-http, andz-schema; clearing that needs anode-zendeskupgrade, which would also free the remaining nock calls infxa-auth-server, allowing us to remove altogether.unit test currently failing due to unrelated terms/privacy change