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

🐛 BUG: unstable_dev fetch cannot accept query param #2641

Closed
WalshyDev opened this issue Jan 29, 2023 · 1 comment · Fixed by #2957 · May be fixed by BlobDevelopment/workers-tracing#13
Closed

🐛 BUG: unstable_dev fetch cannot accept query param #2641

WalshyDev opened this issue Jan 29, 2023 · 1 comment · Fixed by #2957 · May be fixed by BlobDevelopment/workers-tracing#13
Labels
bug Something that isn't working

Comments

@WalshyDev
Copy link
Member

WalshyDev commented Jan 29, 2023

Which Cloudflare product(s) does this pertain to?

Wrangler

What version of Wrangler are you using?

2.8.1

What operating system are you using?

Mac

Describe the Bug

Due to the code here:
https://github.com/cloudflare/wrangler2/blob/89d78c0ac444885298ac052b0b3de23b69fb029b/packages/wrangler/src/api/dev.ts#L302-L328

The URL that's passed into fetch for unstable_dev does not handle query params. This obviously causes any test using these to fail.

Repro:

import { writeFile } from 'fs/promises';
import { expect, test } from 'vitest';
import { unstable_dev } from 'wrangler';

test('unstable_dev fetch repro', async () => {
	const script = `
	export default {
		fetch(req) {
			const url = new URL(req.url);
			const name = url.searchParams.get('name');
			return new Response('Hello, ' + name);
		}
	}
	`;
	await writeFile('script.js', script, { encoding: 'utf8' });

	const worker = await unstable_dev('script.js');

	const res = await worker.fetch('http://worker?name=Walshy');

	expect(res.status).toBe(200);

	const text = await res.text();

	expect(text).toBe('Hello, Walshy');

	await worker.stop();
});

Error:

 FAIL  test/repro.test.ts > unstable_dev fetch repro
AssertionError: expected 'Hello, null' to be 'Hello, Walshy' // Object.is equality
 ❯ test/repro.test.ts:25:15
     23|  const text = await res.text();
     24|
     25|  expect(text).toBe('Hello, Walshy');
       |               ^
     26|
     27|  await worker.stop();

  - Expected   "Hello, Walshy"
  + Received   "Hello, null"

A fix I'd suggest is instead just relying on the URL object, something like:

const url = new URL(input);
url.protocol = `${protocol}:`;
url.hostname = readyAddress;
url.port = readyPort;
input = url.toString();

Instead of manually constructing the URL string.

@WalshyDev WalshyDev added the bug Something that isn't working label Jan 29, 2023
@bretthoerner
Copy link

bretthoerner commented Mar 16, 2023

Related/additionally, it doesn't unpack a Request at all if it's passed in, so even changing the path has no effect if you use the template:

const req = new Request('http://falcon', { method: 'GET' });
const resp = await worker.fetch(req);

i.e. If you change that to new Request('http://falcon/foo', { method: 'GET' }); your app receives http://ip:port/ as the request.url

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something that isn't working
Projects
None yet
2 participants