Feature toggle request: Drop unhandled network requests instead of bypassing them

[amir-ziaei]

Scope

Adds a new behavior

Compatibility

• This is a breaking change

Feature description

I am delighted with how the msw library warns me about unhandled network requests, as shown in the example below:

console.warn
      [MSW] Warning: captured a request without a matching request handler:
      
        • GET http://example.com/some-path
      
      If you still wish to intercept this unhandled request, please create a request handler for it.
      Read more: https://mswjs.io/docs/getting-started/mocks

However, I would like to propose an additional opt-in feature: I suggest the option to automatically drop any network requests that do not have a matching handler by default. By enabling this feature, devs will be required to provide a handler for all requests, resulting in more deterministic tests.

I would love to hear your opinions and thoughts on this.

[kettanaito]

Hi, @amir-ziaei. Thanks for the proposal!

Can you please tell me more about what you mean by "drop any network requests"?

We do have something that sounds very similar to your proposal, that being the "error" value of the onUnhandledRequest option:

server.listen({ onUnhandledRequest: 'error' })

This will throw an exception whenever a request happens that doesn't have a corresponding request handler. Would this address your concern?

[kettanaito]

Generally speaking, my opinion on the deterministic handling of requests is that there must only be one place where you/the library can affect the request resolution: that's request handlers. So unless you have a request handler that explicitly says "whenever X request happens abort it" (which we do support), MSW must not affect the request resolution in any way. This kind of restricted responsibility makes it easy to reason about request behavior and observe what handlers you have at any given point of time that may affect it.

Even the onUnhandledRequest option is a side-effect during a request resolution. I believe setting it to the "error" value will have no effect on the request itself but the current process will throw. This is what you want during tests to prevent unhandled requests from ever being made "silently".

I don't think there's any intention to change this approach but there's a few things around it that can contribute to a better DX, such as #951.

[RobinClowers]

This is a problem for my project as well. I have an API a bit like graphql, where all requests are posts to the same endpoint, and the body determines the result type. I have a wrapper function that uses the http.post function and matches on request body. If it does not match, we return undefined, so other handlers will run. However, if all handlers do not match, we want to get an error, not try and make a real request.

[kettanaito]

The way handler lookup works right now, if you have a matching handler, MSW considers the request as handled:

msw/src/core/utils/handleRequest.ts

Lines 82 to 90 in 7df2533

	// If the handler lookup returned nothing, no request handler was found
	// matching this request. Report the request as unhandled.
	if (!lookupResult.data) {
	await onUnhandledRequest(request, options.onUnhandledRequest)
	emitter.emit('request:unhandled', { request, requestId })
	emitter.emit('request:end', { request, requestId })
	handleRequestOptions?.onPassthroughResponse?.(request)
	return
	}

Even if your handler returned nothing, the last matching handler will stay associated with the request, and treated as one (responsible) for handling it.

This is intentional behavior that trails back to v1, where there was no way to disambiguate between handlers that explicitly returned undefined (i.e. "please keep searching for other handlers") or those that implicitly returned undefined ("oops, I forgot to handle the request").

The mock response API has been redesigned since, and now all your response handling actions are explicit:

• Return/throw a Response instance to use it as a mocked response;
• Return undefined earlier to short-circuit this handler;
• Return passthrough() to opt-out from further handler lookup.

I think we may implement what you are describing if we also treat the request unhandled if it had a handler but that handler didn't return any instructions (basically, returned undefined). If that would tap into onUnhandledRequest() callback, you would be able to error those requests as I described in my answer above.

Wdyt?

[RobinClowers]

Just to make sure I understand your proposal: returning undefined would trigger onUnhandledRequest, rather than the current passthrough behavior? That would be perfect for me, my only concern is users who might be relying on the old behavior.