Event Handler - Response Validation w/ coerced schema

[nateiler]

First, @svozza thank you for all your work in the addition to req/res validation!

I briefly brought this up while working on this PR, but the schema response validation doesn't seem to play nicely w/ schemas that coerce the result.

Let's take this schema:

z.object({
  id: z.coerce.string(),
  name: z.string(),
});

I would expect if it were used as the response body validation schema that the results would reflect the schema. For example:

it('validates a coerced response successfully', async () => {
  // Prepare
  const responseSchema = z.object({
    id: z.coerce.string(),
    name: z.string(),
  });

  app.get(
    '/users/:id',
    () => {
      return { id: 123, name: 'John' };
    },
    {
      validation: { res: { body: responseSchema } },
    }
  );

  const event = createTestEvent('/users/123', 'GET', {});
  event.pathParameters = { id: '123' };

  // Act
  const result = await app.resolve(event, context);

  // Assess
  expect(result.statusCode).toBe(200);
  expect(result.body).toEqual('{"id":"123","name":"John"}');
});

This test fails because the validation doesn't actually mutate the response. At the moment, is only way to use the parsed/validated response is with another middleware that does the mutation?

I don't think this is a bug as the docs clearly state the intent "[Validated data is available via reqCtx.valid.res and is fully typed based on your schema.](Validated data is available via reqCtx.valid.res and is fully typed based on your schema.)" however I personally find response validation useless as the majority of schemas either coerce (a Date object to string) or omit internal attributes not meant for the response payload.

[dreamorosi]

Hi @nateiler thanks for opening this discussion.

@svozza we discussed the stated limitation/assumption so I agree with the assessment above irw the fact that it's not a bug in itself however I'm not sure about this:

This test fails because the validation doesn't actually mutate the response

What do you think? At first look I think mutating should be supported, but I'd like to hear your opinion on whether there are tradeoffs I'm missing.

[nateiler]

Thanks @dreamorosi,

@svozza I suppose there are a couple paths forward:

1. We leave the code as-is, and update the docs to illustrate the mutate scenario and how to best work with it (add your own middleware)
2. We add mutate sort of flag that is part of the validation prop; when enabled the response is updated to use the mutated response.
3. By default we update the response (and provide the original response in the context similar to what's happening with reqCtx.valid.res)

@rcaughtlaf - and feedback on your end? This should apply to both the body and header response validations

[svozza]

Apologies folks, I've been on PTO the past week and only catching up now. Imo, I don't think it is the responsibility of a validation layer to mutate the data that it is assessing so I would would most be amenable to option 1) as outlined by @nateiler. I am, however, willing to listen to strong arguments or prior art here to change my mind but I can't think of any other web framework that will change your responses in this way. I agree that runtime response validation is somewhat niche, for me the best way to handle responses is to simply use return types and get compilation guarantees given that, unlike requests, responses are controlled by the server:

const createTodoSchema = z.object({ title: z.string() });

type Todo = { id: string; title: string; completed: boolean };

app.post(
  '/todos',
  (reqCtx): Todo => { 
    const { title } = reqCtx.valid.req.body;
    return { id: '123', title, completed: false };
  },
  {
    validation: { req: { body: createTodoSchema } }, 
  }
);

[rcaughtlaf]

responsibility of a validation layer

IMO, this a Standard Schema layer, not just a validation layer. It would be confusing that only a subset of Standard Schema functionality gets "supported".

I agree that runtime response validation is somewhat niche, for me the best way to handle responses is to simply use return types and get compilation guarantees given that, unlike requests, responses are controlled by the server

A couple of things:

• An edge-case, but I could imagine any typed data sources benefiting from response run-time validation. Also schema logic such as min/max provide more granular validation than native types.
• OpenAPI generation would ideally be derived from Standard Schemas, so using it for both request and response definitions seems desirable.

For requests and responses, I personally think that anything under the .valid scope should inherit the fully resolved Standard Schema types (including coercion). If we don't do this (for requests especially), people will have to implement coercion, validation of that coercion and exception handling within their handler or add superfluous custom middlewares. Ideally, the raw req and res objects would be available under a separate scope.

[svozza]

IMO, this a Standard Schema layer, not just a validation layer. It would be confusing that only a subset of Standard Schema functionality gets "supported" [...] or requests and responses, I personally think that anything under the .valid scope should inherit the fully resolved Standard Schema types (including coercion). If we don't do this (for requests especially), people will have to implement coercion, validation of that coercion and exception handling within their handler or add superfluous custom middlewares.

These are very compelling arguments that have convinced me to change my mind. Are we saying then that if a user is using request validation that the response that goes to the client is what's in reqCtx.valid.res.body? As validation is ultimately implemented as middleware, we can over-write reqCtx.res with reqCtx.valid.res.body as you would with any other middleware here:

powertools-lambda-typescript/packages/event-handler/src/http/middleware/validation.ts

Line 46 in fc1f036

if (resSchemas) {

.

[nateiler]

I read the position as; the reqCtx.req and reqCtx.res should contain the parsed Standard Schema (coerced) and we provide the original req and res in something like reqCtx.raw.*. No additional middleware needed. Ultimately, it's the inverse of what's happening now where reqCtx.valid.* contains the parsed Standard Schema (coerced) and the reqCtx.req and reqCtx.res are raw.

[svozza]

We're not adding any middleware though, the validation is middlware already. Also, removing valid and replacing it with raw and the other changes would be a breaking change.

[nateiler]

I think we're all on the same page of no additional middleware.

What if we left valid as-is and added raw? valid act as an indicator that validation was present (like it does now). raw would give other middleware access to the original req/res. We're adding more to the ctx which probably isn't ideal but just throwing around some ideas.