zod icon indicating copy to clipboard operation
zod copied to clipboard

Add the value received to the error message for a discriminated union's type

Open danalexilewis opened this issue 2 years ago • 3 comments

I ran into a small problem with the error reporting that just meant I was having to chase a needle in a haystack. My situation: I am writing a zod definition to represent the data coming back from an api. It is an array of messages each with a type so a discriminated union is perfect.

The issue I ran into was that I had a large json file which was a dump of an api request and as I started defining the messages I would find I would be missing some types. These just became painful to find as I a. didn't know what they were called and b. ended up counting items in an array to find the right position based on the path in the z.parse error :(

In @alexxander original code comment in #792 the value was shown in the error message Received ${val[discriminator]} thus:

ctx.addIssue({
    code: z.ZodIssueCode.custom,
    message: `Invalid discriminator value. Expected one of: ${validDiscriminatorValues.join(', ')}. Received ${val[discriminator]}.`,
    path: [discriminator],
});

In the code that ended up being merged it was done like this:

if (!option) {
  addIssueToContext(ctx, {
    code: ZodIssueCode.invalid_union_discriminator,
    options: this.validDiscriminatorValues,
    path: [discriminator],
   // TODO Add received here?
  });
  return INVALID;
}

I'm happy to put up a PR for this (if anyone can see how to do this please feel free) - I suppose before I start on this journey into understanding the source code (which is surprisingly delightfully terse btw) I just wanted to sanity check 2 things:

  1. Is this something that maintainers would be happy to have added to the codebase - ie maybe there is an opinion that error messages should nat have the value received in them.
  2. Is this possible - ie can addIssueToContext have a received: this.value added to it? and if so is the value available in thisat this point in the code

Thanks again for everyone contributing to this project - you make my life better

danalexilewis avatar Jun 07 '22 18:06 danalexilewis

Hi @agentlewis, as you say I had it implemented (and the implementation was rather trivial), but I was asked to remove it in this comment: https://github.com/colinhacks/zod/pull/899#issuecomment-1026281830

The reason is that zod is trying not to leak possibly sensitive data (such as passwords) into the error messages. Should this sort of functionality be needed, it is suggested that it is implemented outside zod.

alexxander avatar Jun 07 '22 21:06 alexxander

@alexxander ah I see the comment thanks.

hmm ok - I get the logic, but in this case we are talking about the ‘type’ of the discriminated union which though it’s technically a value it’s nature is that it behaves path like. So this may be a case for an exception to the policy of not putting values into error messages.

That at least is my take on it.

I could see a simple map function which checks for unknown types and throws an error if it finds one before I z.parse the data. But this just feels a bit messy. Thanks I’ll use that strategy for now and wait and see if any of the maintainers find my above point interesting.

danalexilewis avatar Jun 07 '22 22:06 danalexilewis

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

stale[bot] avatar Aug 06 '22 22:08 stale[bot]

the explanation of hiding sensitive data seems like a separate concern. that should be controlled by an option given to the parse method or schema definition, or some application level log middleware. when i declare a schema with .enum, i get the received value, why should this be different?

rhyek avatar Oct 13 '23 22:10 rhyek

While this issue has a wontfix label, I'd like to propose to re-open it.

It is extremely tedious to debug these kind of errors without received value in the error message.

As mentioned before, the enum behaves the same way...

caillou avatar Jan 26 '24 09:01 caillou

Just got bit by this myself. It was tough to find in my case. If security is a concern, it would be nice to have a ZodVerboseDiscriminatedUnion that reported more of the details when an error occurs

james2doyle avatar Feb 13 '24 18:02 james2doyle