Conditionally disable SSE in mockServiceWorker

[ThaNarie]

Scope

Improves an existing behavior

Compatibility

• This is a breaking change

Feature description

This is related to #156 and #834.

Note that i'm using https://github.com/mswjs/msw-storybook-addon, but I believe the core "issue" lies in this library.
I'm also using version 2 of MSW currently.

This request comes from the following scenario:

• A normal SSE / EventSource request doesn't need a CORS pre-flight request (OPTIONS), so the server isn't configured to support that, for the project we're working on.
• When MSW / Service Worker intercepts the request in self.addEventListener('fetch', even when configuring globally as bypass, or in an handler as passthrough, there is a fetch request executed inside the Service Worker.
• This fetch request inside the Service Worker cause the browser to first do an OPTIONS preflight, which fails.
• This then causes the event stream to disconnect, reconnect, and so the cycle continues.

I presume that the philosophy of MSW is to make this as transparent as possible, and not require extra server configuration when MSW is turned on.

---

I could patch the mockServiceWorker.js with something like suggested in the other issue:

  if (request.headers.get('Accept') === 'text/event-stream') {
    return;
  }

However, even if this was added as "configuration option for msw", it would completely disallow mocking some connections in some handlers. Especially in storybook, where MSW is inited from a global location, these options could not be changed for different stories (that can only update the current active handlers).

It seems like that as soon as event.respondWith is called, no matter what happens in the registered handlers, the normal browser fetch will NOT execute.

For this "feature", I would like to say in a handler – case by case – wether to use respondWith, or to do return and let the original browser request happen.

If the handler execution is done before the respondWith, this might be possible?
So the handler can return something specific (like ignore instead of passthrough).

event.respondWith(handleRequest(event, requestId));

const handledRequest = handleRequest(event, requestId)
if (handledRequest === ....) {
  event.respondWith(handledRequest);
}

I do see the issue here, that handleRequest can be async, and event.respondWith must be called synchronously in the event handler (i presume), and that supporting this would require some re-architecture.

Happy to hear your thoughts on this!

[kettanaito]

Hi. This is an interesting use case, I would like to learn more about it.

The thing is, the fetch listener must be synchronous. But awaiting for the potential mocked response is async (messaging is async by design + response resolvers can be async).

The only way to achieve asynchronicity in the fetch listener is to operate in a promise passed to respondWith. Hope this clarifies why it's structured that way.

Passthrough should not result in an extra request though. I believe if we return in a promise, the worker will continue with executing that request as-is. Can you please verify that?

[kettanaito]

Based on the source code, passthrough translates to extra fetch indeed right now:

msw/src/mockServiceWorker.js

Lines 186 to 195 in 657edd8

	function passthrough() {
	const headers = Object.fromEntries(requestClone.headers.entries())
	// Remove internal MSW request header so the passthrough request
	// complies with any potential CORS preflight checks on the server.
	// Some servers forbid unknown request headers.
	delete headers['x-msw-intention']
	return fetch(requestClone, { headers })
	}

I wonder if simply returning (resolving the respondWith promise with nothing) would result in a similar behavior to prevent duplicate requests (those are technically not duplicates though).

[kettanaito]

Based on the FetchEvent.respondWith() docs, the promise given to the .respondWith() function must resolve to a Response instance, otherwise a network error will be thrown.

This means that MSW cannot short-circuit the request faster in any way—it's not allowed by the platform. I'm afraid there's not much we can do here.

[ThaNarie]

I can see that any client communication (or even getting the relevant client), requires async logic, so it's not possible to do that in the handler.

Would this behaviour (that EventSource is requiring CORS as soon as you install the service worker) something that should be documented, so others won't be surprised by it in the future?

---

Purely as a train of thought, a potential solution could be something like:

• Allow passing a list of regular expressions, that test the full url (incl query string), that should disable/ignore the fetch handler altogether.
• This list can be passed to the setup/listen initially, and updated later
• This list is then sent to the Service Worker, stored in a Map for each clientId
• Then inside the fetch-handler inside the worker, this map can be checked synchronously, eliminating the need to send messages to the client.

Unsure if this is desired, since it could result in race conditions (request being executed before the configuration is available in the worker), will add more code to the worker file (which you might want to keep as lean as possible).

But maybe you would like to think it over :)