Regression: Response serializer no longer replaces response

[roni-frantchi]

What are the steps to reproduce this issue?

Create a serializer that returns an object.

What happens?

The serializer returned object is set on the body.

What were you expecting to happen?

The returned object should replace the entire response object as the documentation states:

"...If an object is returned, the entire response object is replaced. This is useful if you want to manipulate headers or add additional attributes in the Lambda response."

(and as it was in v1)

Any other comments?

Seems to me like the behavior had changed since this change

I think the only reason the pre-existing unit test did not break was because the response object itself was mutated.

What versions of software are you using?

Node.js Version: 14

Middy Version: 2.0.1

[willfarrell]

How did you get 7 +1 in 3h? Anyways, I should be able to take a look later today.

[willfarrell]

I see the regression,

v1.x

const result = s.serializer(handler.response)

    if (typeof result === 'object') {
      // replace response object if result is object
      handler.response = result
    } else {
      // otherwise only replace the body attribute
      handler.response.body = result
    }

v2.x

request.response.body = s.serializer(request.response)

I do recall simplifying because any object doesn't mean it will be an acceptable response for http. How about this:

    const result = s.serializer(handler.response)
    if (result?.body) {
      handler.response = result
    } else {
      // otherwise only replace the body attribute
      handler.response.body = result
    }

This should give you the flexibility you need while ensuring the correct response structure is maintained. Let me know if this works for you, I can add it in.

[roni-frantchi]

@willfarrell that will work just as well I think, thank you.

[willfarrell]

Will be in the next release. v2.1.0
Thanks for reporting

[RaeesBhatti]

It would be great if this is released soon because it's currently breaking our flow. Thanks!

[willfarrell]

It will likely be released this week. I'm just waiting on feedback for a new middleware.

[roni-frantchi]

Thanks for the prompt release @willfarrell.
Seems like there's one case I've missed which I think what you've suggested won't be good enough.
I'm thinking of cases where the response would be something like 204 - in that case I'd expect the response not to have a body.
Maybe we'd do better to be looking for statusCode rather than body?...

[roni-frantchi]

Or perhaps - check if hasOwnProperty('body') so that one may body: undefined

[roni-frantchi]

OK so it's worse than that @willfarrell - if one has false as their body, the behavior is also not what you would expect.

I think we would have to check for hasOwnProperty('body')

[willfarrell]

v2.1.1 is on it's way