Passed query parameters are completely ignored

[oliversalzburg]

Bug Description

When you pass a query object to intercept() it is only respected if the mocked path is of type string, otherwise the entire query config is silently discarded:

undici/lib/mock/mock-interceptor.js

Lines 73 to 81 in 3a77cbb

	if (typeof opts.path === 'string') {
	if (opts.query) {
	opts.path = buildURL(opts.path, opts.query)
	} else {
	// Matches https://github.com/nodejs/undici/blob/main/lib/fetch/index.js#L1811
	const parsedURL = new URL(opts.path, 'data://')
	opts.path = parsedURL.pathname + parsedURL.search
	}
	}

Thus, setting query in these cases has no effect and all requests will be captured, regardless of their search parameters.

Reproducible By

Expected Behavior

Only requests that are matched by the query object are mocked. Ideally, query would accept a callback function, just like the other options.

Logs & Screenshots

Environment

Debian 12, Node 20

Additional context

[mcollina]

Thanks for reporting! Would you like to send a Pull Request to address this issue? Remember to add unit tests.

[oliversalzburg]

I wouldn't mind, but I think I was mostly confusing myself, because I was trying to port some existing nock code.

Now, I just handle path matching with a callback. This callback receives the path with the potential query attached, so I can just .split("?") it off, parse, and apply any kind of matching on it that I want. So the query option in its current form seems to be soley sugar for buildUrl when providing a string URL and the query as a plain hash. As this is the only case where the query option is used, it feels a bit redundant to me. I feel like consumers should just call buildUrl themselves if they want to construct a path that includes query parameters, and then just have a single location (the path matching callback) to handle the matching consistently.

That seems reasonable, compared to implementing an entire new code path for query parameter matching in undici. It would also raise the question if consumers should even receive query parameters as part of the path in the future, as there would be a dedicated path for query matching then. That also sounds like a breaking change that's probably not worth it.

So, maybe just a small addition to the documentation to mention that the query option only applies to string URLs and that the path, when supplied as a RegExp or a callback, will receive the path with query parameters attached as input? Because I think that was just unclear to me.

Let me know what you think 😊

[metcoder95]

Would you like to send a PR for it?

[oliversalzburg]

Yeah, I'll try to make it happen this weekend :)