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

parseMultipartData don't work with File class in Node.js #947

Closed
chentsulin opened this issue Oct 24, 2021 · 8 comments · Fixed by #1436
Closed

parseMultipartData don't work with File class in Node.js #947

chentsulin opened this issue Oct 24, 2021 · 8 comments · Fixed by #1436
Labels
bug Something isn't working scope:node Related to MSW running in Node

Comments

@chentsulin
Copy link
Contributor

Environment

Name Version
msw 0.35.0
node 16.11.1
OS OSX 10.15.3

Request handlers

// Example of declaration. Provide your code here.
import { setupServer } from 'msw/node'
import { rest } from 'msw'

const server = setupServer(
  rest.post(
    'https://api-data.line.me/v2/bot/audienceGroup/upload/byFile',
    (req, res, ctx) => {
      console.log(req);
      return res(
        ctx.json({
          audienceGroupId: 1234567890123,
          createRoute: 'MESSAGING_API',
          type: 'UPLOAD',
          description: 'audienceGroupName_01',
          created: 1613698278,
          permission: 'READ_WRITE',
          expireTimestamp: 1629250278,
          isIfaAudience: false,
        })
      );
    }
  ),
)

server.listen()

Actual request

import fs from 'fs';
import FormData from 'form-data';

const file = fs.createReadStream(
   path.resolve(`${__dirname}/fixtures/audiences.txt`)
 );

const form = new FormData();
form.append('description', 'description');
form.append('file', file, {
  contentType: 'text/plain',
 });

await axios.post('https://api-data.line.me/v2/bot/audienceGroup/upload/byFile', form, {
   headers: form.getHeaders(),
 });

Current behavior

It throws ReferenceError: File is not defined in this line:

: new File([contentBody], filename, { type: contentType })

Expected behavior

I expect there is a File class polyfill or something that works in Node.js

Screenshots

The error catched in try/catch block:

@chentsulin chentsulin added bug Something isn't working scope:node Related to MSW running in Node labels Oct 24, 2021
@chentsulin
Copy link
Contributor Author

chentsulin commented Nov 11, 2021

I found a File class polyfill may help in this case: https://github.com/node-fetch/fetch-blob

Maybe we should have something like this:

if (!globalThis.File) {
  globalThis.File = await import('fetch-blob/file.js')
}

Note: It's a ESM only module.

@chentsulin
Copy link
Contributor Author

chentsulin commented Nov 19, 2021

There is a cjs version of File class in this package: https://github.com/octet-stream/form-data

A workaround by now:

// TODO: use `File` from fetch-blob >= 3.x after upgrading to ESM.
import { File as FileClass } from 'formdata-node';

// for TS users:
declare global {
  var File: typeof FileClass;
}

if (!globalThis.File) {
  globalThis.File = FileClass;
}

@chentsulin
Copy link
Contributor Author

Another blocker is #929.

@delijah
Copy link

delijah commented Mar 5, 2022

We are running into the same issue. Any progress here?

@kettanaito
Copy link
Member

Hey, @chentsulin. Thanks for reporting this.

I agree that we shouldn't rely on the File class that exists only in the browser. There may be a different way to handle the multipart body parsing, perhaps omitting the usage of File altogether.

That being said, I don't have the time capacity to look into this. I encourage anybody reading this (perhaps @delijah?) to debug this and see why the File instance is needed and how we can replace it with something environment-agnostic. I'm not a fan of polyfills so we should strive towards a unified solution.

@delijah
Copy link

delijah commented Mar 8, 2022

I've investigated some more here....

  • It might be possible to use File and FormData from jsdom (if this is anyway already a dependency)
  • Or, what we are using at the moment:
const extractFilesFromMultipartBody = (req) =>
    new Promise((resolve) => {
        const buffer = Buffer.from(req.body, 'hex');
        const bb = busboy({ headers: req.headers.all() });
        const files = [];

        bb.on('file', (name, file, info) => {
            const { filename, mimeType } = info;
            const buffers: Buffer[] = [];

            file.on('data', (data) => {
                buffers.push(data);
            }).on('close', () => {
                const buffer = Buffer.concat(buffers);

                files.push({
                    buffer,
                    name: filename,
                    type: mimeType,
                });
            });
        });

        bb.on('close', () => {
            resolve(files);
        });

        bb.end(buffer);
    });

It looks like msw is currently converting req.body to a hex-string for multipart requests, when running in a node.js environment.

@MartinJaskulla
Copy link
Contributor

MartinJaskulla commented May 2, 2022

In a way @chentsulin's issue is not limited to just FormData or File.

Similar issues might occur for other body types:

type BodyInit = Blob | BufferSource | FormData | URLSearchParams | ReadableStream<Uint8Array> | string;

There are three challenges

  1. Service workers only transmit text, so msw needs to do custom parsing so that the user receives something usable as the request argument in their RequestHandler. In some cases the browser even modifies the request made by the user's code. For example when the user makes a POST request with FormData as the body, the browser will add the Content-Type header automatically. msw then needs to parse the body back into an object.
  2. msw should behave similar in Node and the browser
  3. Node is missing is browser interfaces such as File, FormData etc.

Instead of having msw doing custom parsing or polyfilling, it could make sense to pass the original values the user has passed to fetch/axios back to the user's RequestHandler.

For example in the issue description @chentsulin is already using the form-data library. We could save the original body (form) and the original headers from this request (by overwriting fetch)...

await axios.post('https://api-data.line.me/v2/bot/audienceGroup/upload/byFile', form, {
   headers: form.getHeaders(),
 });

...and reuse them when creating the request which is passed to RequestHandler

// parseWorkerRequest
const request: RestRequest = {
// parseIsomorphicRequest
const mockedRequest: MockedRequest = {

This means msw could remove some of its custom parsing logic and does not need any polyfills in Node, users can use whatever polyfill they want e.g.form-data and use the objects created by their polyfill library in their RequestHandler.

I have written in more detail about this here:
#1188 (comment)

As I have written in the linked comment I don't really know what I am talking about here 😄

Usage example

Without headers

await axios.post('url', form);

Browser

rest.post("/post", async (req, res, ctx) => {
  // req.body is the original body (native browser FormData). The modified body (with boundary value) returned from the service worker is ignored
  // req.headers has the Content-Type header which was added by the browser (The header includes a now useless boundary value. Useless because msw is not using it to parse the modified body)
})

Node

rest.post("/post", async (req, res, ctx) => {
  // req.body is the original body (whatever FormData polyfill the user installed)
  // req.headers is empty as Node does not add headers automatically like the browser does (?)
})

With headers

await axios.post('url', form, {headers: form.getHeaders()});

Browser

rest.post("/post", async (req, res, ctx) => {
  // req.body is the original body (native browser FormData). The modified body (with boundary value) returned from the service worker is ignored
  // req.headers is the original headers
})

Node

rest.post("/post", async (req, res, ctx) => {
  // req.body is the original body (whatever FormData polyfill the user installed)
  // req.headers is the original headers
})

@kettanaito
Copy link
Member

Released: v2.0.0 🎉

This has been released in v2.0.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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working scope:node Related to MSW running in Node
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants