Use @mswjs/interceptors for mocking - WIP #2517

mikicho · 2023-09-11T19:22:11Z

Why?

Decompose interception and mocking code. The need for fundamental change is not a new concept, and efforts have been made toward it. 🚧 Gregor's code review and refactor #2252
Node 18+: make nock work native fetch #2397. @mswjs/interceptors supports both http.get/request and fetch.
Nock in the Browser? :)
Nock is a veteran who has accumulated numerous patches over the years and supports ancient versions of node. @mswjs/interceptors is inspired by Nock and written in a modern way for maximum compatibility.
Focus the maintenance on one robust interception library.
Prevent issues like this: Support both msw and nock levino/mock-jwks#207

closes #2397, closes #2183

Parity Gaps

Nock is a mature and battle-proof project and used by a lot of developers over the years. therefore, Nock covers many real-life scenarios and takes care of a LOT of edge cases.
@mswjs/interceptors is great, but it still has some gaps to cover in order to reach Nock's level of maturity and ability to cover its wide test suite.

I adjust as much as I can Nock's code to @mswjs/interceptors logic and current capabilities and opened tickets in the repo to track issues and feature requests:

Topics to discuss about

During the work, I gathered a few topics we need to discuss further. I'll open dedicated issue(s) for them in Nock's repo.
CC @gr2m @kettanaito

gr2m · 2023-09-14T18:02:54Z

Sorry I'm not sure when I get to review it, I'll try to find time on Saturday.

If we manage to replace nock's on interceptors with the one from @mswjs while keeping all tests passing then I'm all for it

lib/intercept.js

kettanaito · 2023-09-18T16:01:27Z

lib/intercept.js

-        return overriddenRequest(options, callback)
+        const req = convertFetchRequestToClientRequest(request);
+        req.on('response', response => {
+          request.respondWith(new Response('test', { status: 200 }))


Nitpick: we'd want to read the response stream to the ReadableStream of the response instance and call request.respondWith() on the end event of the original response here.

As I mentioned in the issue, let me know if exporting this existing utility from Interceptors would help here. I think it would.

Do you have an idea how we can wait for the nockResponse end event? I use a promise, but maybe you had something else in mind.

As I mentioned in the issue, let me know if exporting this existing utility from Interceptors would help here. I think it would.

Thanks!! For now, I copied the file. We can update this later.

In this case, we can rely on the internal http.ClientRequest implementation to guarantee the order of event execution.

const stream = new Readable() const fetchResponse = new Response(stream, { status: response.statusCode, statusText: response.statusMessage, headers: response.headers // pseudo-code }) response.on('data', (chunk) => stream.write(chunk)) response.on('end', () => { stream.finish() request.respndWith(fetchResponse) })

This is a gist. If you want the actual implementation example, take a look at this function.

kettanaito · 2023-09-18T16:02:10Z

lib/intercept.js

-      )
+  overrideClientRequest()
+  const interceptor = new BatchInterceptor({
+    name: 'my-interceptor',


Giving this a meaningful name may improve the logging output, afaik. Maybe name: 'nock-interceptor'?

kettanaito · 2023-09-18T16:03:10Z

lib/intercept.js

+    host: url.hostname,
+    port: url.port || (url.protocol === 'https:' ? 443 : 80),
+    path: url.pathname + url.search,
+    headers: fetchRequest.headers,


I think Node expects OutgoingHeaders here, which is not compatible with the Headers instance. You should be fine doing this:

headers: Object.fromEntries(fetchRequest.headers.entries())

great catch! 🙏

Just be cautious about the compilation target here. I'm not sure what's the support range is for Nock, but .fromEntries() and .entries() methods may not exist on super old versions of ECMAScript and Node.

kettanaito · 2023-09-18T18:10:06Z

lib/create_response.js

+ * @param {IncomingMessage} message 
+ */
+function createResponse(message) {
+  const readable = new ReadableStream({


Looks like the version of Node Nock uses doesn't have the ReadableStream global. I'd double-check if this isn't the linter's problem as the first measure.

Sometimes the linter doesn't play well with relatively newly added globals. This can be modified in the linter's globals key, if talking about ESLint.

Thanks!
I think we can fix this after we will solve the got error. Most of Nock's tests are based on it and I'm eager to solve it and run all tests to find cases that we haven't covered yet.

I will try to look into the got issue this week based on my availability. Right now, the best next step is to reliably reproduce it in an automated test. I've already written one (see https://github.com/mswjs/interceptors/pull/432?notification_referrer_id=NT_kwDOAOSmz7M3NzYwMzQ3MDA1OjE0OTg0OTEx&notifications_query=is%3Aunread#issuecomment-1724139617) but you've provided some input that I need to reflect in the test.

I've fixed the got issue, would appreciate a quick review of the pull request!

mikicho · 2023-09-20T20:29:13Z

@kettanaito
Until we resolve the got problem, I workaround the problem in my local machine and work on fixing the failing test 💪

Maybe you have an idea of how I can propagate errors from the interceptors? Specifically, I'm trying to fix this test. I tried something like:

interceptor.on('request', function ({ request, requestId }) {
  ...
  const nockRequest = convertFetchRequestToClientRequest(request);
  nockRequest.on('error', error => {
    request.respondWith(new Response('put here the real error'))
    resolve()
  })
  ...
})

But it stuck.

kettanaito · 2023-09-20T21:32:12Z

@mikicho, you can throw in the request listener but that will be considered a request error. Try it, see if it produces the result you need.

interceptor.on('request', () => {
  nockRequest.on('error', (error) => {
    throw error
  })
})

mikicho · 2023-09-20T21:39:51Z

@kettanaito
The test fails with Uncaught Error, I'd expect msw to catch the error and emit an error event, which later gets caught by the listener in the test

kettanaito · 2023-09-21T10:32:02Z

Oh, I see. Try responding with Response.error()?

request.respondWith(Response.error(YOUR_ERROR_MESSAGE))

This will instruct Interceptors to treat this as an intentional error, and it will forward it to the appropriate error handling logic based on the request module, like emitting the error event in case of http.ClientRequest.

mikicho · 2023-09-21T22:46:26Z

test_intercept almost done! Most of the failures have a PR in @msw/interceptors repo, but a couple still need to be reviewed and addressed. We will handle them soon. :)

Next test file, I'm coming 🥣

mikicho · 2023-09-23T10:25:59Z

We are making progress and moving forward!

mikicho · 2023-09-24T10:59:09Z

💪

gr2m · 2023-09-24T20:24:17Z

Just want to say thank you @mikicho and @kettanaito for working on this 💐💝

kettanaito · 2023-09-24T21:09:06Z

@gr2m, all praise is rightfully @mikicho's! I'm happy that Nock's test suite helps us uncover missing bits to the Interceptors and improve the library. All I want is the best interception for the end developer and this collaboration helps achieve just that.

mikicho · 2023-09-25T22:04:34Z

@kettanaito Thank you for your willingness to help and I apologize for any clutter I may cause!
With your assistance, we can support both the 'nock' and 'msw' approaches to mocking with a robust foundation!

mikicho · 2023-09-26T19:14:58Z

@gr2m Why does Nock expose the request object via the response ("res.req")
https://github.com/nock/nock/blob/main/tests/test_define.js#L232
This is not part of the IncomingMessage object API.

mikicho · 2023-09-26T20:16:21Z

@gr2m Also, we currently pass the rawHeaders as-is. according to the docs, the rawHeaders is string of array.
While we support arrays, objects, and maps in the reply function, we don't stringify the values.
WDYT? This is a breaking change. I think it is a bug, and the response is invalid and misleading.

kettanaito · 2023-09-26T21:09:48Z

Why does Nock expose the request object via the response ("res.req")

I believe it's not part of the public API but Node.js associates the OutgoingMessage with the IncomingMessage that way. I do recall seeing something like that recently during testing, so you may be seeing the same thing. It's highly likely Node's internals.

gr2m · 2023-10-26T17:05:26Z

@mikicho your personal well being and your loved ones always come first ❤️

mikicho · 2023-11-24T12:55:44Z

😁

piotrmorek · 2023-12-14T08:51:40Z

Hey @mikicho are you going to progress with your PR? Amazing work btw. 💪
We at our company looking forward to using nock supporting fetch from node.

It's a blocker for us right now - because of usage of swagger-js which uses undici from this version

Looking forward for this PR to be ready-to-go and merged soon 🚀

kettanaito · 2023-12-14T10:02:11Z

A quick update on my side: I'm on holiday until January. I will come back then and will plan all the open issues/pull requests to @mswjs/interceptors into my workflow so to unblock this feature. Huge thanks for improving the library!

mikicho · 2023-12-15T11:06:48Z

@piotrm221 Thanks! :) There are some things to do in both nock and mswjs/interceptors. I hope we can merge this in January.

mikicho · 2023-12-16T18:00:53Z

gr2m · 2024-01-14T18:38:57Z

@mikicho @kettanaito Thank you for continuing to work on this! Would it help if I onboarded you as a @nock maintainer? I'm having a lot going on at work and life right now. I expect it to calm down in the 2nd half of the year, I can't wait to find more time for Open Sourcering again. But until then, I don't want to become a bottleneck, and native fetch support is essential if nock is to have a future

kettanaito · 2024-01-14T20:28:24Z

@gr2m, I will do my best to help with what I can. I'm back from the holidays and slowly getting back to open source. I have "Nock + Interceptors" on my roadmap, we will get it done. Your test suite has already pointed out multiple shortcomings of our implementation, so it's essential for MSW's future as well!

I think making @mikicho a maintainer would certainly ease the organization around merging and publishing this change 💯

mikicho · 2024-01-14T20:28:26Z

I think there is not a lot of work to make this happen, and this work contributes to both Nock and MSW.
@kettanaito, do you have time for the last push to make this work?

kettanaito · 2024-01-14T20:30:00Z

@mikicho, I will have time. I'm a bit busy right now but this change is a goal I want to see finished. I'm prioritizing it alongside my other work in MSW. I think we've addressed most of the issues related to the 2.0 release of the library, so I can switch to other things, this change included! Need to go through all your open issues and pull requests.

If you can join my Discord we could iterate on this faster. My GitHub notifications tend to get overwhelming.

mikicho · 2024-03-13T20:41:08Z

Quick update:

Thanks to @kettanaito's work, we now have a socket-based interceptor, which should be more future-proof and more compatible with Node.
There are some tests to fix and things to resolve between the libraries, but hopefully, we could make the shift for mswjs/interceptors soon 🤞

mikicho · 2024-03-30T12:50:11Z

mikicho · 2024-04-13T16:15:13Z

wip

576b296

wip

94b9b90

mikicho mentioned this pull request Sep 17, 2023

Node 18+: make nock work native fetch #2397

Open

2 tasks

wip

ef59cf6

kettanaito reviewed Sep 18, 2023

View reviewed changes

lib/intercept.js Outdated Show resolved Hide resolved

kettanaito reviewed Sep 18, 2023

View reviewed changes

mikicho added 2 commits September 18, 2023 20:15

fixes

d7d17bd

fix

855914e

kettanaito reviewed Sep 18, 2023

View reviewed changes

mikicho mentioned this pull request Sep 21, 2023

Handle errors in the interceptor handler mswjs/interceptors#437

Closed

wip

51dd0c6

mikicho added 2 commits September 22, 2023 02:51

more fixes

21cf9c2

more fixes

091d876

more fixes

79f5162

MaxMood96 mentioned this pull request Sep 11, 2023

[Snyk] Upgrade sinon from 15.0.4 to 15.2.0 MaxMood96/nock#73

Closed

mikicho added 3 commits December 15, 2023 14:29

wip

cbd31cb

Merge remote-tracking branch 'upstream/main' into Michael/fetch-support

bc4327a

wip

7df18f2

mikicho added 3 commits December 17, 2023 22:48

fix

53e3d2a

fix

ca9e616

more-tests

2c0e4f1

prince-chrismc mentioned this pull request Dec 25, 2023

Bump @actions/github from 5.1.1 to 6.0.0 prince-chrismc/label-merge-conflicts-action#790

Open

shubhbapna mentioned this pull request Dec 27, 2023

feat: Migrate to undici MockClient kiegroup/mock-github#79

Closed

tamil777selvan mentioned this pull request Jan 2, 2024

V9 migrate from got to fetch webdriverio/webdriverio#11942

Merged

8 tasks

wip

bcbdd04

This was referenced Feb 3, 2024

feat: native fetch mocking #2580

Merged

Code assumes clearImmediate #2398

Closed

mikicho mentioned this pull request Feb 25, 2024

fix: replace debug with node debug #2592

Merged

wip

ab8037d

fix

e7a6309

kettanaito mentioned this pull request May 11, 2024

Implement Service Worker API nodejs/undici#3246

Open

12 tasks

Use @mswjs/interceptors for mocking - WIP #2517

Are you sure you want to change the base?

Use @mswjs/interceptors for mocking - WIP #2517

Conversation

mikicho commented Sep 11, 2023 • edited

Why?

Parity Gaps

Topics to discuss about

gr2m commented Sep 14, 2023

Choose a reason for hiding this comment

mikicho Sep 18, 2023 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

mikicho Sep 18, 2023 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

kettanaito Sep 21, 2023 • edited

Choose a reason for hiding this comment

mikicho commented Sep 20, 2023

kettanaito commented Sep 20, 2023

mikicho commented Sep 20, 2023

kettanaito commented Sep 21, 2023

mikicho commented Sep 21, 2023

mikicho commented Sep 23, 2023

mikicho commented Sep 24, 2023 • edited

gr2m commented Sep 24, 2023

kettanaito commented Sep 24, 2023

mikicho commented Sep 25, 2023

mikicho commented Sep 26, 2023 • edited

mikicho commented Sep 26, 2023 • edited

kettanaito commented Sep 26, 2023

gr2m commented Oct 26, 2023

mikicho commented Nov 24, 2023

piotrmorek commented Dec 14, 2023

kettanaito commented Dec 14, 2023

mikicho commented Dec 15, 2023

mikicho commented Dec 16, 2023

gr2m commented Jan 14, 2024

kettanaito commented Jan 14, 2024

mikicho commented Jan 14, 2024

kettanaito commented Jan 14, 2024 • edited

mikicho commented Mar 13, 2024

mikicho commented Mar 30, 2024

mikicho commented Apr 13, 2024

mikicho commented Sep 11, 2023 •

edited

mikicho Sep 18, 2023 •

edited

mikicho Sep 18, 2023 •

edited

kettanaito Sep 21, 2023 •

edited

mikicho commented Sep 24, 2023 •

edited

mikicho commented Sep 26, 2023 •

edited

mikicho commented Sep 26, 2023 •

edited

kettanaito commented Jan 14, 2024 •

edited