mswjs / interceptors

Low-level network interception library.
https://npm.im/@mswjs/interceptors
MIT License
537 stars 123 forks source link

[XHR] Response interceptors executes after application receives the response #400

Closed JAspeling closed 2 months ago

JAspeling commented 1 year ago

Issue Title

XHR Response interceptors get executed after the application receives the response.

Description

I expect to see the response interceptor functions execute before the application receives the response. This is not the case with XMLHttpRequestInterceptor. It is, however, the case with FetchInterceptor

Context

I am working on a solution where we need to intercept all HTTP requests fired by any client (fetch, Angular, Axios, etc). There's no telling which underlying HTTP method is used to make the request, so it should cater to all.

We came across this @mswjs/interceptors package that looked promising as it allows us to intercept all requests, but in the POC, we encountered some strange behaviour.

The expected flow would be:

HTTP request initiated

  - Request interceptor 1
  - Request interceptor 2
  - Request interceptor ...n

  - Response interceptor 1
  - Response interceptor 2
  - Response interceptor ...n

HTTP Response received by the application

This should be the same for all HTTP requests.

Current Behavior

I've noticed that XHR requests it does not follow the above pattern. Instead, they do the following:

HTTP request initiated

  - Request interceptor 1
  - Request interceptor 2
  - Request interceptor ...n

HTTP Response received by the application

  - Response interceptor 1
  - Response interceptor 2
  - Response interceptor ...n

Expected Behavior

I expect the interceptor functions to be executed in the same order, whichever HTTP method is used.

Request interceptor functions should be executed before the request reaches the API, and Response interceptor functions should be executed before the response reaches the application.

Screenshots and Additional Information (if applicable)

image

Steps to Reproduce (if applicable)

Please see the reproduction repo where the below behaviour can be seen: Github Repo: https://github.com/JAspeling/mswjs-interceptors-angular/blob/main/src/main.ts Stackblitz: https://stackblitz-starters-dgrmdd.stackblitz.io Stackblitz editor: https://stackblitz.com/edit/stackblitz-starters-dgrmdd?file=src%2Fmain.ts

Environment

Angular CLI: 16.1.6
Node: 18.17.0
Package Manager: npm 9.6.7
OS: win32 x64

@angular-devkit/architect       0.1601.6
@angular-devkit/build-angular   16.1.6
@angular-devkit/core            16.1.6
@angular-devkit/schematics      16.1.6
@angular/cli                    16.1.6
@schematics/angular             16.1.6
rxjs                            7.8.1
typescript                      5.1.6
zone.js                         0.13.1

Please feel free to reach out if anything is unclear or if there is a misunderstanding around using the @mswjs/interceptors package.

JAspeling commented 1 year ago

@kettanaito Any idea why I'm getting this behaviour?

kettanaito commented 1 year ago

Hey, @JAspeling. Thanks for a great issue report. Let me provide some additional context and let's see whether we're on the same page.

Intentions

I can confirm that the intention of this library is to emit the request event when the request is sent, and the response event once the response is received. This stands true for all currently implemented interceptors with the exception for the ClientRequestInterceptor which I will mention below.

Regarding the FetchInterceptor, it emits the response event after your application receives a response. What may be confusing here is that the .then() callback this library adds will execute before your own .then() callbacks.

https://github.com/mswjs/interceptors/blob/36400ccc47bf53b269888a7a3a5f452df592f58b/src/interceptors/fetch/index.ts#L99-L111

But that's not what's happening. These two callbacks will execute one after another, but both happen once the response has been received (the request promise has been resolved). It is the Promise-like nature of handling fetch responses that may be confusing here.

It's important to note that response received isn't equivalent to response read. This stands true for fetch and for ClientRequest also, where the response event is emitted as soon as the response is received (the response event of ClientRequest) but the actual response body is streamed into the Fetch API response instance you can access in the response event listener:

https://github.com/mswjs/interceptors/blob/36400ccc47bf53b269888a7a3a5f452df592f58b/src/interceptors/ClientRequest/NodeClientRequest.ts#L249-L254

Technically speaking, we should emit the request event earlier than we do now, and then pipe the request body chunks to the ReadableStream of the Fetch API request instance representing the in-progress request. I think that would contribute to a more predictable behavior, but it will also change the intention because the request event would be emitted when the request is constructed.

Regarding XMLHttpRequest

We emit the response event within the load event of the XMLHttpRequest, which indicates a successful fetch. You see the difference in time between the response event being emitted and your own onreadystatechange callback because state change triggers before the load event does. Here's an excerpt from the [XMLHttpRequest specification](https://xhr.spec.whatwg.org/#the-send()-method) describing this order:

  1. Fire an event named readystatechange at xhr.
  2. Fire a progress event named load at xhr with transmitted and length.
  3. Fire a progress event named loadend at xhr with transmitted and length.

Because of this order, your callback will always execute before the response event gets emitted by this library.

I would love to move the response event to readyStateChange once it received response headers, which would translate to the first byte of the response. The challenge here is that, to my best knowledge, there is no method in XHR to read incoming response body chunks. That reading seems to be done internally and all the consumer can do is monitor the bytes read in the progress event (but not the bytes themselves). This makes it problematic to represent the response body as a ReadableStream the response event consumers would be able to read as the response body comes in.

Summary

This is mainly for myself, but let's write down the current state of things:

With this in mind, the behavior of the request event seems a bit inconsistent. We should consider changing when the request event is emitted in ClientRequestInterceptor and XMLHttpRequestInterceptor to be aligned with how it's emitted in the FetchInterceptor, that being:

The response event is consistent except for the XMLHttpRequestInterceptor, as you've rightfully pointed out. We should change it to be emitted as soon as request received headers (readyState 3), create a ReadableStream to represent the response body chunks as they come in, and close the stream on loadend (I think it should be safer there to account for errors while reading the response; loadend always executes).

My only concern is whether we'd be able to represent the incoming XHR response body as a stream. I wonder if the value of .response will gradually change in the progressevent. I didn't find the specification mentioning this but I might have overlooked it.

Do you have any insight regarding the reading of the XHR response body? That'd be great.

kettanaito commented 1 year ago

If we can find a way to represent the incoming XHR response body as ReadableStream, I'm for this change. I may even have a minute this week to implement it. The research seems to be the biggest challenge here. I could use some help with it.

JAspeling commented 1 year ago

Thank you for the detailed explanation! I will take the discussion up with my team and perhaps try to get them involved as well - maybe we can find a solution together. I'll do some research on the subject πŸ‘

smo043 commented 3 months ago

@kettanaito could you pls help with a fix? I am encountering the same issue. The response is sent before the interceptor request is completed.

kettanaito commented 2 months ago

Released: v0.32.0 πŸŽ‰

This has been released in v0.32.0!

Make sure to always update to the latest version (npm i @mswjs/interceptors@latest) to get the newest features and bug fixes.


Predictable release automation by @ossjs/release.