Problem with query params #1300

csbenjamin · 2021-09-21T03:20:59Z

Reproduction

Steps to reproduce the behavior:

When using node-fetch version 2.6.3, making a request to http://nginx/transactions/total?month=2021-09 do not send the url params. If I am using node-fetch version 2.6.1 everything work fine. Enabling node http verbose log, when making the request using node-fetch version 2.6.1 I get:

HTTP 52: createConnection nginx:80: {
  protocol: 'http:',
  slashes: true,
  auth: null,
  host: 'nginx',
  port: 80,
  hostname: 'nginx',
  hash: null,
  search: '?month=2021-09',
  query: 'month=2021-09',
  pathname: '/api/transactions/total',
  path: null,
  href: 'http://nginx/api/transactions/total?month=2021-09',
  method: 'GET',
  headers: [Object: null prototype] {
    Authorization: [
      'Bearer ....'
    ],
    Accept: [ '*/*' ],
    'User-Agent': [ 'node-fetch/1.0 (+https://github.com/bitinn/node-fetch)' ],
    'Accept-Encoding': [ 'gzip,deflate' ],
    Connection: [ 'close' ]
  },
  agent: undefined,
  servername: 'nginx',
  _agentKey: 'nginx:80:'
}

but using node-fetch version 2.6.3 I get

HTTP 24: createConnection nginx:80: {
  path: null,
  pathname: '/api/transactions/total',
  hostname: 'nginx',
  protocol: 'http:',
  port: 80,
  hash: '',
  search: '?month=2021-09',
  query: undefined,
  href: 'http://nginx/api/transactions/total?month=2021-09',
  method: 'GET',
  headers: [Object: null prototype] {
    Authorization: [
      'Bearer ....
    ],
    Accept: [ '*/*' ],
    'User-Agent': [ 'node-fetch/1.0 (+https://github.com/bitinn/node-fetch)' ],
    'Accept-Encoding': [ 'gzip,deflate' ],
    Connection: [ 'close' ]
  },
  agent: undefined,
  host: 'nginx',
  servername: 'nginx',
  _agentKey: 'nginx:80:'
}

Note the query part is undefined when using node-fetch version 2.6.3.

Expected behavior

The behaviour of the two versions should be the same.

Screenshots

Your Environment

software	version
node-fetch	2.6.3
node	14.17.3
npm	6.14.3
Operating System	linux

Additional context

The text was updated successfully, but these errors were encountered:

JasonYeMSFT · 2021-09-21T04:33:50Z

+1 We encountered this bug in our project as well. In the parseURL function, the path property needs to carry the query string since it is so far the only way to send query parameters to the node standard https/http.request API.

jimmywarting · 2021-09-21T07:42:02Z

Sorry about this, looking into it

see node-fetch/node-fetch#1300

simllll · 2021-09-21T11:38:37Z

We have the same issue, the issue is that whatwgUrl.URL does not return a "query" parametr, only a "search", e.g.:

{
    path: '/youtube/v3/playlistItems',
    pathname: '/youtube/v3/playlistItems',
    hostname: 'youtube.googleapis.com',
    protocol: 'https:',
    port: '',
    hash: '',
    search: '?part=id&part=snippet&part=status&playlistId=PLeLcJvyWxh4jXXCA4jIPH0g6usuZHfnGz&maxResults=50',
    query: undefined, <-- this is undefined, but shouldn't
    href: 'https://youtube.googleapis.com/youtube/v3/playlistItems?part=id&part=snippet&part=status&playlistId=PLeLcJvyWxh4jXXCA4jIPH0g6usuZHfnGz&maxResults=50'
  }

Also interesting is that watwg-url is avaiable in version 9, but the referenced version is ^5.0.0. On my monorepo setup, version 9 is installed though, I guess this could also be related?

ace7536

pimterry · 2021-09-21T13:22:45Z

v2.6.4 was just released, and fixes this for me 👍

jimmywarting · 2021-09-21T13:25:07Z

Yea, released it as a hot fix without merging #3001

did not announce it or closed this issue,
#1301 should be merged/accepted

Exilz · 2021-09-21T14:13:53Z

v2.6.4 fixed this for us too. The latest version broke our app in production 🥲

LinusU · 2021-09-21T14:28:35Z

Very sorry for the breakage everyone!

A fix has been pushed out as version 2.6.4 and we have added a test case so that this will not break again in the future.

Thank you @jimmywarting for the quick fix ❤️

I would recommend everyone to use package lock files to avoid having your production break, that way you can test your app before any dependencies change.

Fix for node-fetch/node-fetch#1300

jimmywarting · 2021-09-21T14:43:31Z

Already release, closing now as it got merged

LinusU · 2021-09-21T14:47:44Z

@jimmywarting would you be able to deprecate version 2.6.3 on npm?

jimmywarting · 2021-09-21T14:49:44Z

@jimmywarting would you be able to deprecate version 2.6.3 on npm?

Something in lines of: npm deprecate [email protected] "critical bug fixed in v2.6.4"? is that right?

LinusU · 2021-09-21T14:51:53Z

Sounds good 👍

or maybe:

npm deprecate [email protected] "query parameters being dropped bug fixed in v2.6.4"

jimmywarting · 2021-09-21T14:54:36Z

Or after #1303 is merged:

npm deprecate [email protected] "Query parameters being dropped. Bug fixed in v2.6.5"

LinusU · 2021-09-21T15:00:28Z

Hmm, that specific bug was fixed in 2.6.4 though, I think that people will just upgrade to the latest version anyways? And I don't think that nearly as many people are affected by #1303, which also manifests as a crash on startup so no subtle behaviour...

But anyways, no strong feelings from my side so you can decide 👍

LinusU · 2021-09-21T15:21:50Z

Some more info on what and why this happened:

I was back porting #701 to the v2.x branch, specifically this line:

https://github.com/node-fetch/node-fetch/pull/701/files#diff-19dd46c0a2527b1db65f8b69bf935c70e64cd34db0f1d8f3c2fb1b92bf7a467fR270

But what I failed to realise that it had a follow up PR later that fixed the very same bug that I now introduced:

https://github.com/node-fetch/node-fetch/pull/759/files#diff-19dd46c0a2527b1db65f8b69bf935c70e64cd34db0f1d8f3c2fb1b92bf7a467fR263

This is the issue that previously reported this for the 3.x release line: #749

#759 didn't add any test cases, but I think that a good thing to do now is to port the test case added in the hot fix here to the 3.x branch, and to backport all other tests from 3.x to 2.x to further ensure that we don't do any breaking changes...

…ring (node-fetch/node-fetch#1300)

csbenjamin added the bug label Sep 21, 2021

D4N14L mentioned this issue Sep 21, 2021

Lock node-fetch to avoid breaking change in 2.6.3 version microsoft/rushstack#2917

Merged

jimmywarting mentioned this issue Sep 21, 2021

fix v2.6.3 that did not send query params #1301

Merged

4 tasks

shazron added a commit to adobe/aio-lib-test-proxy that referenced this issue Sep 21, 2021

fix: pin node-fetch to 2.6.2 (bug)

4a1f365

see node-fetch/node-fetch#1300

jimmywarting mentioned this issue Sep 21, 2021

Authentication parameters in url does not work on 2.6.3 #1302

Closed

andrei-tatar mentioned this issue Sep 21, 2021

Node-RED failing to update Smart-Nora with devices andrei-tatar/node-red-contrib-smartnora#102

Closed

kryops mentioned this issue Sep 21, 2021

npm i [email protected] failing with 403 response status Automattic/node-canvas#1902

Closed

thgreasi mentioned this issue Sep 21, 2021

Update balena-request to v11.4.2 balena-io/balena-sdk#1126

Merged

4 tasks

timaiv mentioned this issue Sep 21, 2021

keyvault getSecret stopped working last night in Azure Webservice on Linux Azure/azure-sdk-for-js#17759

Closed

3 tasks

dlenroc mentioned this issue Sep 21, 2021

fix: maximum redirects reached in [email protected] dlenroc/node-testrail-api#35

Merged

OllysCoding mentioned this issue Sep 21, 2021

Update DCR Dependencies (workspaces prep) guardian/dotcom-rendering#3443

Merged

whatwewant mentioned this issue Sep 21, 2021

node-fetch v2.6.3 request missing query string zcorky/fz#4

Closed

ciffelia added a commit to ciffelia/bluetail that referenced this issue Sep 21, 2021

Upgrade indirect dependency node-fetch

f34a301

Fix for node-fetch/node-fetch#1300

jimmywarting closed this as completed Sep 21, 2021

gr2m mentioned this issue Sep 21, 2021

pin version of node-fetch and add end-to-end test to catch breaking changes octokit/request.js#432

Closed

ehmicky mentioned this issue Sep 22, 2021

My build stopped working with @netlify/build 18.11.2 netlify/build#3649

Closed

whatwewant added a commit to zcorky/fz that referenced this issue Sep 23, 2021

chore: node-fetch is not stable, v2.6.3 merge a big bug with query st…

7bfa683

…ring (node-fetch/node-fetch#1300)

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Problem with query params #1300

Problem with query params #1300

csbenjamin commented Sep 21, 2021 •

edited

Loading

JasonYeMSFT commented Sep 21, 2021 •

edited

Loading

jimmywarting commented Sep 21, 2021

simllll commented Sep 21, 2021

pimterry commented Sep 21, 2021

jimmywarting commented Sep 21, 2021

Exilz commented Sep 21, 2021 •

edited

Loading

LinusU commented Sep 21, 2021

jimmywarting commented Sep 21, 2021

LinusU commented Sep 21, 2021

jimmywarting commented Sep 21, 2021

LinusU commented Sep 21, 2021

jimmywarting commented Sep 21, 2021

LinusU commented Sep 21, 2021

LinusU commented Sep 21, 2021

Problem with query params #1300

Problem with query params #1300

Comments

csbenjamin commented Sep 21, 2021 • edited Loading

JasonYeMSFT commented Sep 21, 2021 • edited Loading

jimmywarting commented Sep 21, 2021

simllll commented Sep 21, 2021

pimterry commented Sep 21, 2021

jimmywarting commented Sep 21, 2021

Exilz commented Sep 21, 2021 • edited Loading

LinusU commented Sep 21, 2021

jimmywarting commented Sep 21, 2021

LinusU commented Sep 21, 2021

jimmywarting commented Sep 21, 2021

LinusU commented Sep 21, 2021

jimmywarting commented Sep 21, 2021

LinusU commented Sep 21, 2021

LinusU commented Sep 21, 2021

csbenjamin commented Sep 21, 2021 •

edited

Loading

JasonYeMSFT commented Sep 21, 2021 •

edited

Loading

Exilz commented Sep 21, 2021 •

edited

Loading