Skip to content
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

tests: Replace use of preq with node-fetch #253

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

Conversation

jdforrester
Copy link
Member

Bug: T293853
Bug: T309772

@santhoshtr
Copy link
Member

If we increase the node version to 18(LTS, Maintainance mode) or later, 'fetch' API is standardized and stable in node. No need for an external library.
(cxserver just migrated away from preq using node native fetch https://phabricator.wikimedia.org/T350773)

@jdforrester
Copy link
Member Author

If we increase the node version to 18(LTS, Maintainance mode) or later, 'fetch' API is standardized and stable in node. No need for an external library. (cxserver just migrated away from preq using node native fetch https://phabricator.wikimedia.org/T350773)

Yes, but other parts of the library aren't yet compatible with Node 18, so we can't do that right now (until I or someone else has the time to fix those incompatibilities).

@mvolz
Copy link
Collaborator

mvolz commented May 27, 2024

Not sure what's causing the errors here, but as I'm looking into preq for an unrelated issue, I thought I should note it differs from fetch in at least two ways:

Preq is wrapping errors, so we might loose some logging features upstream if those are not replaced upstream;

Preq uses another defunct library called requestretry to automatically retry requests once after a short time out.

@jdforrester
Copy link
Member Author

Not sure what's causing the errors here, but as I'm looking into preq for an unrelated issue, I thought I should note it differs from fetch in at least two ways:

Preq is wrapping errors, so we might loose some logging features upstream if those are not replaced upstream;

Preq uses another defunct library called requestretry to automatically retry requests once after a short time out.

Yeah, those two are things to bear in mind for downstream users if they're just preq in prod code, but given this is tests-only here I think it should be fine. But then, it's breaking tests (or something else is), so… :-(

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
3 participants