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

fix(rest): support ReadableStream as response body #974

Closed
wants to merge 1 commit into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
45 changes: 43 additions & 2 deletions src/mockServiceWorker.js
Original file line number Diff line number Diff line change
Expand Up @@ -203,6 +203,10 @@ async function getResponse(event, client, requestId) {
)
}

case 'MOCK_STREAM_START': {
return respondWithStream(clientMessage)
}

case 'MOCK_NOT_FOUND': {
return getOriginalResponse()
}
Expand Down Expand Up @@ -305,8 +309,28 @@ function sendToClient(client, message) {
const channel = new MessageChannel()

channel.port1.onmessage = (event) => {
if (event.data && event.data.error) {
return reject(event.data.error)
if (event.data) {
if (event.data.error) {
return reject(event.data.error)
}

switch (event.data.type) {
// message 'MOCK_STREAM_START' resolves the Promise, but then the client sends
// more messages from a ReadableStream (see issue #581)
case 'MOCK_STREAM_CHUNK': {
if (event.data.payload) {
globalThis.currentStreamController.enqueue(
event.data.payload.chunk,
)
}
return
}

case 'MOCK_STREAM_END': {
globalThis.currentStreamController.close()
return
}
}
}

resolve(event.data)
Expand All @@ -329,6 +353,23 @@ function respondWithMock(clientMessage) {
})
}

function respondWithStream(clientMessage) {
const stream = new ReadableStream({
start(controller) {
globalThis.currentStreamController = controller
},
}).pipeThrough(
// from https://web.dev/fetch-upload-streaming/#streaming-request-bodies
// > Each chunk of a [response] body needs to be a Uint8Array
new TextEncoderStream(),
)

return new Response(stream, {
...clientMessage.payload,
headers: clientMessage.payload.headers,
})
}

function uuidv4() {
return 'xxxxxxxx-xxxx-4xxx-yxxx-xxxxxxxxxxxx'.replace(/[xy]/g, function (c) {
const r = (Math.random() * 16) | 0
Expand Down
3 changes: 3 additions & 0 deletions src/setupWorker/glossary.ts
Original file line number Diff line number Diff line change
Expand Up @@ -78,6 +78,9 @@ export type ServiceWorkerOutgoingEventTypes =
*/
export type ServiceWorkerFetchEventTypes =
| 'MOCK_SUCCESS'
| 'MOCK_STREAM_START'
| 'MOCK_STREAM_CHUNK'
| 'MOCK_STREAM_END'
| 'MOCK_NOT_FOUND'
| 'NETWORK_ERROR'
| 'INTERNAL_ERROR'
Expand Down
29 changes: 29 additions & 0 deletions src/utils/worker/createRequestListener.ts
Original file line number Diff line number Diff line change
Expand Up @@ -46,6 +46,35 @@ export const createRequestListener = (
})
},
onMockedResponse(response) {
if (response.body instanceof ReadableStream) {
channel.send({
type: 'MOCK_STREAM_START',
payload: { ...response, body: '__mocked_stream__' },
})

const reader = response.body.getReader()
const sendChunk = () => {
reader.read().then(({ done, value }) => {
if (done) {
channel.send({
type: 'MOCK_STREAM_END',
})
} else {
channel.send({
type: 'MOCK_STREAM_CHUNK',
payload: {
chunk: value,
},
})
sendChunk()
}
})
}
sendChunk()

return
}

channel.send({
type: 'MOCK_SUCCESS',
payload: response,
Expand Down
24 changes: 24 additions & 0 deletions test/rest-api/response/body/body-stream.mocks.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,24 @@
import { setupWorker, rest } from 'msw'

const stream = new ReadableStream({
start(controller) {
controller.enqueue('line1\n')

setTimeout(() => {
controller.enqueue('line2\n')
controller.close()
}, 0)
},
})

const worker = setupWorker(
rest.get('/sse', (req, res, ctx) => {
return res(
ctx.status(200),
ctx.set('Content-Type', 'text/plain'),
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I believe that such a response must also specify that its body is a stream.

Copy link
Sponsor Contributor Author

@Aprillion Aprillion Nov 8, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

apparently not necessary for a stream of text/plain, at least in the integration test setup..

ctx.body(stream),
)
}),
)

worker.start()
7 changes: 7 additions & 0 deletions test/rest-api/response/body/body-stream.node.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
/**
* @jest-environment node
*/

test('Node does not support ReadableStream yet', () => {
expect(typeof ReadableStream).toBe('undefined')
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This test needs to be rewritten, it doesn't test anything as of yet.

Copy link
Sponsor Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

the idea was to fail the test if Node adds ReadableStream ¯\_(ツ)_/¯ if you have something in mind that can be actually tested, that would be awesome!

})
17 changes: 17 additions & 0 deletions test/rest-api/response/body/body-stream.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,17 @@
import * as path from 'path'
import { pageWith } from 'page-with'

test('responds with a ReadableStream response body', async () => {
const { request } = await pageWith({
example: path.resolve(__dirname, 'body-stream.mocks.ts'),
})

const res = await request('/sse')
const status = res.status()
const headers = await res.allHeaders()
const text = await res.text()
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We need to ensure that res.text() actually verifies that you can read that response stream once more on the usage surface (assert that we're correctly piping the original response stream into PassThrough in the worker).


expect(status).toBe(200)
expect(headers['content-type']).toBe('text/plain')
expect(text).toBe('line1\nline2\n')
})