Unable to determine whether req.body was sent as valid JSON

[tomalexhughes]

Hi, big fan of msw. But I’m experiencing the following issue:

Describe the bug

I want to return a different response in my resolver depending on whether req.body is valid JSON or not. This is to allow closer imitation of our backend API within our tests.

Previously my test passed despite sending invalid JSON as I always returned a success response, therefore I would like to return an error response from my mock API if the JSON sent is invalid. I believe this is the correct approach as the Request assertions documentation states that we should not be asserting on the body directly and instead include failure scenarios.

I'm currently unable to validate whether the JSON sent is valid as parseBody automatically parses JSON for us or will return the raw object if not parsable, therefore, the following two requests will have the same body in the request resolver.

Valid JSON Request

fetch("/example", {
  method: "POST",
  headers: {
    "Content-Type": "application/json",
  },
  body: JSON.stringify({ foo: 'bar' }),
});

Invalid JSON Request

fetch("/example", {
  method: "POST",
  headers: {
    "Content-Type": "application/json",
  },
  body: { foo: 'bar' },
});

Resolver

rest.post("/example", (req, res, ctx) => {
  const body = req.body; // Will always be the raw object

  // Shamelessly stolen from https://stackoverflow.com/questions/9804777/how-to-test-if-a-string-is-json-or-not
  function isValidJson(jsonInput) {
    try {
      JSON.parse(jsonInput);
    } catch (e) {
      return false;
    }
    return true;
  }
  
  const bodyWasValid = isValidJSON(req.body); // will always be false

  return res(ctx.json({ bodyWasValid }));
});

In this scenario isValidJSON would always return as false because req.body is always the raw object, I understand returning the raw object is a convenience helper and is a desired behaviour but it makes it impossible to imitate our backend API.

Environment

• msw: ^0.21.3
• nodejs: 12.18.3
• npm: 6.14.8

To Reproduce

Use the above resolver with the above payloads.

Expected behavior

As mentioned above I recognise the automatic parsing of req.body is intentional and would be a breaking change if removed.

My initial thoughts should be that req.body should be '[object Object]' if sent as an object that is not valid JSON. If the body was sent as valid JSON then it should remain as the parsed object.

[kettanaito]

Hey, @tomalexhughes. Thanks for reporting this, it's an interesting one.

Strangely, this isn't the behavior I observe in the latest master:

Above you can see that un-stringified request body is indeed available as [object Object] string in the req.body. The properly stringified request body is parsed into an object and passed to the req.body.

The same behavior is on the version 0.21.3. There must be something else to the issue.

Can you please provide a reproduction scenario for this issue? You can use this sandbox to get started, or push a minimal reproduction repository on GitHub. Thanks!

[kettanaito]

Note that the request's body is already implicitly validated for a JSON validity as a part of the request transformation logic internally:

msw/src/utils/request/parseBody.ts

Lines 7 to 22 in 327b384

	export function parseBody(body?: MockedRequest['body'], headers?: Headers) {
	if (body) {
	// If the intercepted request's body has a JSON Content-Type
	// parse it into an object, otherwise leave as-is.
	const hasJsonContent = headers?.get('content-type')?.endsWith('json')
	if (hasJsonContent && typeof body !== 'object') {
	return jsonParse(body) || body
	}
	return body
	}
	// Return whatever falsey body value is given.
	return body
	}

Thus, if you may reason about the request's body validity by checking its Content-Type header and the type of the req.body value.

rest.post('/json', (req, res, ctx) => {
  const isJson = req.headers.get('content-type')?.endsWith('json')
  const isValidJson = isJson && req.body?.constructor.name === 'Object'
})

[tomalexhughes]

Hey, thanks for the reply :)

I added a console.log statement inside my request resolver and noticed that the response was different in my tests compared to the browser.

In the browser req.body is "[object Object]".

However, I am using the same request resolver for my tests in Node and getting a different result.

My testing environment is as follows:

• msw: ^0.21.3
• nodejs: 12.18.3
• react-scripts: 3.4.3
• jest@24.9.0

I'll try and create a reproduction repo over the weekend.

[tomalexhughes]

Reproduction can be found in this repo.

@kettanaito I've also added the isValidJson to the console output in the response resolver, notice in the screenshots how it is false when running in the browser but true when running within the tests in Node. I wonder if this could be because the fetch API is being polyfilled for Node by react-scripts and the discrepancy is there?

[marcosvega91]

Hi @tomalexhughes to send JSON with fetch function you should send data in one of the following types

• ArrayBuffer
• ArrayBufferView (Uint8Array and friends)
• Blob/File
• string
• URLSearchParams
• FormData
  https://developer.mozilla.org/en-US/docs/Web/API/Fetch_API/Using_Fetch

You should parse your object as body: JSON.stringify({foo: 'bar'}), and add an header 'content-type': 'application/json' like you have done. I hope this could help :)

[tomalexhughes]

@marcosvega91 Hey Marco! Thanks for the reply.

I'm aware how to send valid JSON but my issue is that I want to be able to test in my resolver when the request payload is invalid so I can return an error response rather than a success.

Currently when invalid JSON is sent req.body inside the response resolver is a different value in Node than in the browser. If you run the repro app (both in the browser and the test) you will see that when invalid JSON is sent (i.e. it hasn't been stringified) it logs as "[object Object]" however in Node the resolver has the raw object.

I’m aware that Node doesn’t have a native fetch and therefore react-scripts will be polyfilling it for tests (although my assumption would be that the polyfill would be the same in the browser as Node) but I would expect req.body to be the same in both my tests and the browser environment.

[marcosvega91]

Ah, I didn't read your first question.

So, your example is using whatwg-fetch to make fetch working on node.
whatwg-fetch will create an instance of XMLHttpRequest to make the request.

XMLHttpRequest instance has a method send to execute the request. The only parameter of the send is the body that, as for spec, should be

body Optional

• A body of data to be sent in the XHR request. This can be:
• A Document, in which case it is serialized before being sent.
• An XMLHttpRequestBodyInit, which per the Fetch spec can be a Blob, BufferSource, FormData, URLSearchParams, or USVString object.
• null

If no value is specified for the body, a default value of null is used.
The best way to send binary content (e.g. in file uploads) is by using an ArrayBufferView or Blob in conjunction with the send() method.

Looking inside the code of the library I see that when send is called a _bodyInit has been send. _bodyInit will contain the original payload without any transformation. So if you send an object send function will be called with the object.

I'm not sure if it's right that whatwg-fetch could do this. Otherwise we should add some check on node-request-interceptor

[kettanaito]

I think window.fetch and whatwg-fetch behave slightly differently when it comes to the scenario of non-stringified request JSON body. I've raised an issue (JakeChampion/fetch#856) in the whatwg-fetch repo to get more clarification.

@marcosvega91 provided a valid observation: XMLHttpRequest.send gets called with the request body as-is as a part of whatwg-fetch functionality. However, it's unlikely to be handled in the node-request-interception as it would introduce a too big of an assumption over the request body.

I suppose what was meant is for a check as like this:

send(data) {
  if (requestContentTypeIsJson && typeof data !== 'string') {
    coerceData(data) // produce a "window.fetch"-like behavior
  }
}

[kettanaito]

The maintainers of whatwg-fetch have verified this issue, so I've opened a pull request to fix it. Awaits code review from the maintainers.

[kettanaito]

Unfortunately, per whatwg-fetch maintainers, that repository is no longer maintained. They said it's unlikely to get that fix merged. This issue is currently blocked.

[kettanaito]

I see little value in keeping this issue open: there is nothing to be addressed on the MSW side. The issue is in the whatwg-fetch polyfill as it handles a non-stringified JSON body differently than window.fetch in a browser (see #856). As the status of that polyfill is "not maintained", I can suggest you switching to a different polyfill in your tests.

You can still track the resolution progress with whatwg-fetch in this pull request: JakeChampion/fetch#881.

[kettanaito]

The fix on the whatwg-fetch side has been published: https://github.com/github/fetch/releases/tag/v3.6.0.
I suggest updating that dependency and checking if the issue is gone.

[tomalexhughes]

For anyone who comes across this issue, although the issue is now closed, this still seems to be an issue even after the whatwg-fetch update.

Testing environment is:

• msw: 0.28.1
• nodejs: 15.0.1
• react-scripts: 4.0.0
• whatwg-fetch: 3.6.2

@kettanaito I will try and debug this further and create a follow-up on the whatwg-fetch repo or here depending on where the issue resides. I've updated my reproduction repo if this is something you want to look into yourself or if you wish to re-open this issue.

[kettanaito]

Thanks for reporting back, @tomalexhughes. Just to be safe: could you please ensure that both whatwg-fetch and msw are at their latest versions?

[tomalexhughes]

Hey @kettanaito, my versions are as follows:

• msw: 0.28.1
• whatwg-fetch: 3.6.2

I'll attempt to force the resolution to whatwg-fetch 3.6.0 as it looks like 3.6.1 changed some of the code your PR introduced and let you know if 3.6.0 works.

EDIT: Appears that 3.6.2 of whatwg-fetch reverts your original code change with JakeChampion/fetch@e42f201 :/ I'll continue the discussion on the github/fetch repo

[kettanaito]

Thanks for figuring that out, @tomalexhughes. I hope the whatwg-fetch maintainers share some insights as to why that was reverted and we can learn from that to come up with a better solution on their side.