chore: simplify creation of `fetch error` #2327

fedyk · 2023-10-10T18:54:44Z

Simplify creation of `fetch error`

This relates to the way how fetch error is created. TypeError accept cause in constructor, so Object.assign can be removed.

Refs: #1633

* Fix default fetch parameters * Preserve existing behavior with 300 second timeout if not defined * Add test for 300 second timeout as default * Cleanup old unused tests * Simplify how fetch utilizes timeouts from agent

* wpt: make runner more resilient * wpt: cleaner exit, hopefully

* Make test pass in v19.x Signed-off-by: Matteo Collina <hello@matteocollina.com> * fixup Signed-off-by: Matteo Collina <hello@matteocollina.com> * enable v19 Signed-off-by: Matteo Collina <hello@matteocollina.com> * fixup Signed-off-by: Matteo Collina <hello@matteocollina.com> * fixup Signed-off-by: Matteo Collina <hello@matteocollina.com> * github CI timeout Signed-off-by: Matteo Collina <hello@matteocollina.com> Signed-off-by: Matteo Collina <hello@matteocollina.com>

Signed-off-by: Matteo Collina <hello@matteocollina.com> fix #1890

* feat: expose content-type parser * fix: add docs

…meter (#2295)

Bumps [sinon](https://github.com/sinonjs/sinon) from 15.2.0 to 16.1.0. - [Release notes](https://github.com/sinonjs/sinon/releases) - [Changelog](https://github.com/sinonjs/sinon/blob/main/docs/changelog.md) - [Commits](sinonjs/sinon@v15.2.0...v16.1.0) --- updated-dependencies: - dependency-name: sinon dependency-type: direct:development update-type: version-update:semver-major ... Signed-off-by: dependabot[bot] <support@github.com> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>

Fixes #2287

* feat: respect Node.js flag * add test * cleaner test * fix test * update docs * revert empty space * remove already default value * add * don't use `globalThis.fetch` to pass test in Node.js 16 * add test under `test/fetch` * fix lint * import from `undici-fetch` * use correct paths, use `request` for pure undici test * fix Node.js 14 test

KhafraDev · 2023-10-10T20:40:07Z

A few things:

this will drop support for node v16.8 (or make the error entirely useless). Not really an issue but the README should be changed.
there's at least one more place where this pattern is used, I think in lib/fetch/body.js? It should be changed there also.

mcollina

I don't think we should be landing this without bumping a major.

codecov-commenter · 2023-10-11T06:39:48Z

Codecov Report

Attention: 424 lines in your changes are missing coverage. Please review.

Comparison is base (61246a5) 90.34% compared to head (f541cb6) 85.42%.
Report is 250 commits behind head on next.

Additional details and impacted files

@@            Coverage Diff             @@
##             next    #2327      +/-   ##
==========================================
- Coverage   90.34%   85.42%   -4.93%     
==========================================
  Files          70       76       +6     
  Lines        6038     6867     +829     
==========================================
+ Hits         5455     5866     +411     
- Misses        583     1001     +418

Files	Coverage Δ
index-fetch.js	`100.00% <100.00%> (ø)`
lib/api/abort-signal.js	`100.00% <100.00%> (ø)`
lib/api/api-connect.js	`97.87% <100.00%> (+0.09%)`	⬆️
lib/api/api-stream.js	`100.00% <100.00%> (ø)`
lib/cache/symbols.js	`100.00% <100.00%> (ø)`
lib/cookies/index.js	`100.00% <100.00%> (ø)`
lib/cookies/parse.js	`90.90% <100.00%> (-0.30%)`	⬇️
lib/core/connect.js	`80.00% <100.00%> (-13.43%)`	⬇️
lib/core/symbols.js	`100.00% <ø> (ø)`
lib/dispatcher-base.js	`91.57% <100.00%> (+0.08%)`	⬆️
... and 33 more

... and 3 files with indirect coverage changes

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

metcoder95 · 2023-10-11T07:46:17Z

Changed the branch but it seems next needs to be rebased

fedyk · 2023-10-11T07:48:44Z

@KhafraDev thanks for your comment. I've adjusted lib/fetch/body.js and lib/fetch/response.js as well.

As for changes in README, I didn't get the point with dropping support for node v16.8. If it is critical for this MR, please elaborate 🙂

mcollina · 2023-10-11T08:04:32Z

This commit should target the next branch

mcollina · 2023-10-11T08:05:16Z

I merged main in next

KhafraDev · 2023-10-12T17:26:28Z

As for changes in README, I didn't get the point with dropping support for node v16.8. If it is critical for this MR, please elaborate

error.cause was added in node v16.9, at least according to mdn. On node v16.8, which is currently supported by undici, the error message will be completely useless with this change, essentially dropping support for it IMO.

build/wasm.js

lib/fetch/util.js

+// https://w3c.github.io/webappsec-subresource-integrity/#grammardef-hash-with-options
+// https://www.w3.org/TR/CSP2/#source-list-syntax
+// https://www.rfc-editor.org/rfc/rfc5234#appendix-B.1
+const parseHashWithOptions = /((?<algo>sha256|sha384|sha512)-(?<hash>[A-z0-9+/]{1}.*={0,2}))( +[\x21-\x7e]?)?/i


fedyk · 2023-10-15T14:32:25Z

As for changes in README, I didn't get the point with dropping support for node v16.8. If it is critical for this MR, please elaborate

error.cause was added in node v16.9, at least according to mdn. On node v16.8, which is currently supported by undici, the error message will be completely useless with this change, essentially dropping support for it IMO.

You are right - cause is supported since v16.9.0

fedyk · 2023-10-15T14:33:27Z

I've created a new clean MR for the changes #2347. This one got glutted

KhafraDev and others added 30 commits January 18, 2023 21:40

fix: flaky abort test (#1863)

16c4128

fix: flaky abort test

c9d4a6b

5.15.1

9d5f231

fix: don't load websockets w/o crypto support (#1868)

2d2512c

fix: don't include websockets in node bundle (#1869)

22d2f70

fix: don't set socket on client before init

f61a902

Refs: #1633

Fix fetch parameters not being applied correctly (#1870)

d6ba9e5

* Fix default fetch parameters * Preserve existing behavior with 300 second timeout if not defined * Add test for 300 second timeout as default * Cleanup old unused tests * Simplify how fetch utilizes timeouts from agent

fix: typo in contributing.md (#1874)

5883598

chore(performance#38): Performance improvements (#1871)

1813408

perf(util): improve isFormDataLike checks (#1875)

d7602f6

5.15.2

9457c97

Add feature to specify custom headers for proxies (#1877)

69ede10

fix: don't swallow llhttp loading error

d0a99c2

5.16.0

81b1521

fix(wpts): Blob is a global getter in >=v19.x.x (#1880)

bdb1d08

doc: fix anchor links dispatcher.stream (#1881)

f02ab5b

wpt: make runner more resilient (#1884)

25a8aac

* wpt: make runner more resilient * wpt: cleaner exit, hopefully

remove timeout-minutes in github action

196c4da

Signed-off-by: Matteo Collina <hello@matteocollina.com> fix #1890

Correct the type of DispatchOptions["headers"] (#1896)

a9dabd5

perf(content-type parser): faster string collector (#1894)

67e9796

feat: expose content-type parser (#1895)

f4693d6

* feat: expose content-type parser * fix: add docs

Update DispatchOptions type (#1889)

0c75102

fix: update error type definitions (#1888)

055780a

fix: ensure connection header is a string (#1900)

db0b5ca

fix: throw if invalid content-type header (#1901)

2b260c9

fix(fetch): use semicolon for Cookie header delimiter (#1906)

0986381

perf: use FastBuffer (#1907)

208f7cf

5.17.0

16b7a68

fix: bad buffer slice

d2be675

kyrylodolynskyi and others added 9 commits October 5, 2023 14:52

chore: change order of the pseudo-headers (#2308)

898040d

fix: Agent.Options.factory should accept URL object or string as para…

e645012

…meter (#2295)

test: handle npm ignore-scripts settings (#2313)

af78c98

Fixes #2287

fix(#2311): End stream after body sent (#2314)

e5c9d70

disallow setting host header in fetch (#2322)

470ee38

[StepSecurity] ci: Harden GitHub Actions (#2325)

882ff6d

Simplify fetch error creation

1970637

fedyk changed the title ~~Simplify fetch error creation~~ Simplify creation of fetch error Oct 10, 2023

fedyk changed the title ~~Simplify creation of fetch error~~ chore: simplify creation of fetch error Oct 10, 2023

ronag approved these changes Oct 10, 2023

View reviewed changes

metcoder95 approved these changes Oct 10, 2023

View reviewed changes

mcollina requested changes Oct 11, 2023

View reviewed changes

mcollina added the semver-major Features or fixes that will be included in the next semver major release label Oct 11, 2023

fedyk added 2 commits October 11, 2023 09:39

apply same pattern to fetch/body.js

2333b11

adjust fetch/response.js

f541cb6

metcoder95 changed the base branch from main to next October 11, 2023 07:45

github-advanced-security bot found potential problems Oct 13, 2023

View reviewed changes

fedyk mentioned this pull request Oct 15, 2023

simplify error and cause #2347

Closed

7 tasks

fedyk closed this Oct 15, 2023

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

chore: simplify creation of `fetch error` #2327

chore: simplify creation of `fetch error` #2327

fedyk commented Oct 10, 2023 •

edited

Loading

KhafraDev commented Oct 10, 2023

mcollina left a comment

codecov-commenter commented Oct 11, 2023 •

edited

Loading

metcoder95 commented Oct 11, 2023 •

edited

Loading

fedyk commented Oct 11, 2023

mcollina commented Oct 11, 2023

mcollina commented Oct 11, 2023

KhafraDev commented Oct 12, 2023

fedyk commented Oct 15, 2023

fedyk commented Oct 15, 2023

chore: simplify creation of fetch error #2327

chore: simplify creation of fetch error #2327

Conversation

fedyk commented Oct 10, 2023 • edited Loading

Simplify creation of fetch error

KhafraDev commented Oct 10, 2023

mcollina left a comment

Choose a reason for hiding this comment

codecov-commenter commented Oct 11, 2023 • edited Loading

Codecov Report

metcoder95 commented Oct 11, 2023 • edited Loading

fedyk commented Oct 11, 2023

mcollina commented Oct 11, 2023

mcollina commented Oct 11, 2023

KhafraDev commented Oct 12, 2023

fedyk commented Oct 15, 2023

fedyk commented Oct 15, 2023

chore: simplify creation of `fetch error` #2327

chore: simplify creation of `fetch error` #2327

fedyk commented Oct 10, 2023 •

edited

Loading

Simplify creation of `fetch error`

codecov-commenter commented Oct 11, 2023 •

edited

Loading

metcoder95 commented Oct 11, 2023 •

edited

Loading