Msw: [v0.22.x] Regression on parsing response

Created on 20 Nov 2020  路  3Comments  路  Source: mswjs/msw

Describe the bug

Hi again. 馃榾
For what I got the new version now supports the definition of custom transformers to parse the response. From here I see we have a default transformer that will stringify the response body if the content-type header ends with "json". This is a regression from version 0.21.3 since now the ctx.body does not work as expected. From the docs:

ctx.body()
Sets a raw response body. Does not apply any Content-Type headers or other response transformations.

So giving:

rest.get('api/resource/:id', (req, res, ctx) => {
    const status = 400;
    const problemJson = {
       type: 'app-error-code',
       title: 'Mock response provided by msw',
       status,
       detail: 'Mock response provided by msw',
       instance: 'string',
    };
    return res(
      ctx.status(status),
      ctx.body(JSON.stringify(problemJson)),
      ctx.set('Content-Type', 'application/problem+json')
    );
  }),

This will not work because now the default transformer sees that the content-type header ends with "json" and will stringify the body again. That will surround my string body with double quotes: ""MY_STRINGIFIED_BODY"".

Changing the ctx.body(JSON.stringify(problemJson)) to ctx.body(problemJson) works. Before version 0.21.3 it was working as the documentation specifies.

I can work in a PR to change this but it depends on which direction it should go:

  • Just update the docs to make it clear
  • Change the stringifyJsonBody function to not stringify again if the res.body is already a string.

Environment

  • msw: 0.22.1
  • nodejs: 12.19.0
  • npm: 6.14.8

Expected behavior

Avoid stringify the res.body more than once.

bug

Most helpful comment

I believe the way we handled #410 was wrong. Instead of preserving ctx.json as objects in order to merge them and having to ensure it gets stringified on the response level, we should have parsed and merged each individual ctx.data/ctx.errors payloads. The cost of this is small, as you should not have multiple ctx.data or ctx.errors transformers in the same response composition.

When it comes to handling a JSON response body, these points must stand:

  • ctx.json always stringifies the given body.
  • ctx.json sets the value of the _entire_ response body _once_.
  • If you wish to combine multiple JSON pieces either combine them before passing to ctx.json, or create your custom response transformer.
  • Custom parsing/stringification of data that cannot be dealt with via JSON, consider writing a custom response transformer.

All 3 comments

Hey, @otaciliolacerda. Thank you once more for discovering a regression.

Change the stringifyJsonBody function to not stringify again if the res.body is already a string.

This makes sense, as using ctx.body without extra headers (implicit or explicit) should always set whatever you set on the response body, not bothering with the stringification if that's not necessary.

I'd be thankful if you issue a pull request with this fix so we could see how it affects the rest of the functionality.

I will open a PR later today. 馃榾

I believe the way we handled #410 was wrong. Instead of preserving ctx.json as objects in order to merge them and having to ensure it gets stringified on the response level, we should have parsed and merged each individual ctx.data/ctx.errors payloads. The cost of this is small, as you should not have multiple ctx.data or ctx.errors transformers in the same response composition.

When it comes to handling a JSON response body, these points must stand:

  • ctx.json always stringifies the given body.
  • ctx.json sets the value of the _entire_ response body _once_.
  • If you wish to combine multiple JSON pieces either combine them before passing to ctx.json, or create your custom response transformer.
  • Custom parsing/stringification of data that cannot be dealt with via JSON, consider writing a custom response transformer.
Was this page helpful?
0 / 5 - 0 ratings