Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

ReadableStream object could not be cloned when trying to use stream as a mocked response body #581

Closed
BlackGlory opened this issue Feb 5, 2021 · 7 comments · Fixed by #1288
Labels
bug Something isn't working help wanted Extra attention is needed needs:discussion needs:tests

Comments

@BlackGlory
Copy link
Contributor

This issue originated from the earlier issue: #156

If we need msw to support SSE, then we need to make the code work:

export const worker = setupWorker(
  rest.get('/sse', (req, res, ctx) => {
    return res(
      ctx.status(200)
    , ctx.set('Content-Type', 'text/event-stream')
    , ctx.body(sse(function* () {
        yield 'message1'
        yield 'message2'
      }))
    )
  })
)

function sse(gfn) {
  let iter
  return new ReadableStream({
    start() {
      iter = gfn()
    }
  , pull(controller) {
      controller.enqueue(`data: ${iter.next().value}\n\n`)
    }
  })
}

Browser:

[MSW] Request handler function for "GET http://localhost:8080/sse" has thrown the following exception:

DOMException: Failed to execute 'postMessage' on 'MessagePort': ReadableStream object could not be cloned.
(see more detailed error stack trace in the mocked response body)

Node.js:

TypeError: The first argument must be of type string or an instance of Buffer, ArrayBuffer, or Array or an Array-like Object. Received an instance of ReadableStreamTypeError [ERR_INVALID_ARG_TYPE]: The first argument must be of type string or an instance of Buffer, ArrayBuffer, or Array or an Array-like Object. Received an instance of ReadableStream

      at ClientRequestOverride.<anonymous> (node_modules/node-request-interceptor/src/interceptors/ClientRequest/ClientRequestOverride.ts:216:34)
      at step (node_modules/node-request-interceptor/lib/interceptors/ClientRequest/ClientRequestOverride.js:33:23)
      at Object.next (node_modules/node-request-interceptor/lib/interceptors/ClientRequest/ClientRequestOverride.js:14:53)
      at fulfilled (node_modules/node-request-interceptor/lib/interceptors/ClientRequest/ClientRequestOverride.js:5:58)

Originally posted by @BlackGlory in #156 (comment)

@kettanaito kettanaito changed the title Server-sent Events support ReadableStream object could not be cloned when trying to use stream as a mocked response body Feb 5, 2021
@kettanaito
Copy link
Member

Hey, @BlackGlory. Thank you for reporting this.

While the SSE/WebSocket support is in progress (#156 , #396) and are not intended to work at the moment, we must ensure that a ReadableStream can be used as a mocked response body.

Please, would you be interested in creating a failing test suite that illustrates this issue? Your example above looks good. Perhaps we could drop yield and make it as plain as possible. Once we've got a failing test and clearly defined expectations, we can look into why the library doesn't fulfill them. You can follow the contribution guidelines to land your first pull request. Me, or other maintainers, will help you along the way. Let us know if that sounds like something you'd like to do. Thanks.

@kettanaito kettanaito added needs:tests bug Something isn't working and removed feature labels Feb 7, 2021
@BlackGlory
Copy link
Contributor Author

I don't have time to create a test suite for this, I think the example is specific enough, if anyone really wants to advance this feature, it should be enough to reproduce it.

@kettanaito
Copy link
Member

The error originates from the attempt to .postMessage to the worker with the ReadableStream specified as the mocked response's body:

if (port) {
port.postMessage(message)
}

One needs to remember that the ReadableStream instance created in a request handler is created on the client-side, and needs to be sent or otherwise instructed to the worker. As .postMessage transfers a text content, it looks like we'd have to serialize the stream (?), send it in an abstract way, or find an entirely different way to let the worker know it should use a stream as the constructed response's body over here:

function createResponse(clientMessage) {
return new Response(clientMessage.payload.body, {
...clientMessage.payload,
headers: clientMessage.payload.headers,
})
}

From what I can see, we will need to reconstruct the stream in the worker's context, which is unfortunate, as the stream's logic goes beyond the data it transfers but also the way it transfers it. At the moment I don't see a way to propagate a client-side defined ReadableStream to the worker.

@kettanaito kettanaito added the help wanted Extra attention is needed label Mar 8, 2021
@Aprillion
Copy link
Contributor

I managed to create a failing test and I think I found a way to implement some draft solution...

@Aprillion
Copy link
Contributor

Aprillion commented Nov 7, 2021

Note to self: the link to CONTRIBUTING.md was old, new version: https://github.com/mswjs/msw/blob/master/.github/CONTRIBUTING.md

using following steps to access browse console from integration tests:

  • yarn build && DEBUG=pageWith yarn test:integration body-stream.test for logs from createRequestListener.ts
  • in that browser, navigate to chrome://serviceworker-internals/ and click Inspect button for logs from mockServiceWorker.js

@kettanaito
Copy link
Member

Released: v0.43.0 🎉

This has been released in v0.43.0!

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


Predictable release automation by @ossjs/release.

@kettanaito
Copy link
Member

One thing to mention: there is no ReadableStream API in Node. There's Readable, which is a different thing. There are also some transformation utils like Readable.toWeb() in the latest versions of Node but I have no confirmation of whether they construct an actual window.ReadableStream-compatible API.

Generally, we've adopted ReadableStream internally but MSW doesn't have support for ReadableStream as a mocked response body yet. Even this internal change seems to have broken a thing or two for some browsers, as the request is pending indefinitely in some cases (see #1325, #1324).

The way we've implemented readable streams internally is:

  1. Construct new Response(mockedResponse.body) and then read it as response.body.getReader().
  2. Send all read chunks to the worker via messageChannel.postMessage().
  3. On the worker's side, construct another readable stream and manually pipe the incoming messages to that stream's queue:
    switch (event.data.type) {
    case 'MOCK_RESPONSE_CHUNK': {
    streamCtrl.enqueue(event.data.payload)
    break
    }

This effectively means that we're constructing two streams and stitching them together.

I've just tried sending a stream via transport of postMessage but while that works fine in Chrome, browsers like Firefox and Safari refuse to clone a ReadableStream. This is despite the spec describing Transferable as various data types, including ReadableStream.

I suspect that the pending request issue is caused by the operation channel (BroadcastChannel) that we create internally to bind the chunks transfer to a specific request:

// Create a communication channel scoped to the current request.
// This way events can be exchanged outside of the worker's global
// "message" event listener (i.e. abstracted into functions).
const operationChannel = new BroadcastChannel(
`msw-response-stream-${requestId}`,
)

If this channel gets corrupted/destroyed, the internal stream in the worker will pend forever.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working help wanted Extra attention is needed needs:discussion needs:tests
Projects
None yet
3 participants