Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Extensible and Type Discrimination #480

Closed
jrandolf opened this issue Jul 7, 2023 · 3 comments · Fixed by #484
Closed

Extensible and Type Discrimination #480

jrandolf opened this issue Jul 7, 2023 · 3 comments · Fixed by #484

Comments

@jrandolf
Copy link
Contributor

jrandolf commented Jul 7, 2023

While working on Puppeteer, we've noticed it's very difficult to discriminate response types because response types have extensibility. This causes a reduction problem because, e.g., an EmptyResponse can in theory contain the keys of ErrorResponse, but the values are something completely different.

I propose instead of spreading extensibility, we go along with one of the following standard options:

  • Define a private key, say ? extensions: (*text => any), that allows implementation-specific extensions, or
  • Create a registry like how RFCs typically do.

CC: @jgraham @whimboo

@jgraham
Copy link
Member

jgraham commented Jul 10, 2023

Isn't this a fundamental problem if you just look at the CDDL? Per the conversation about intersection types, I think something that matches ErrorResponse would also match EmptyResponse, and so if the assumption is that implementations will in fact add arbitrary extra data to the response guided only by the CDDL requirements, then we can't spec this away.

On the other hand, nothing in the spec currently allows browsers to actually add any extra data to responses. The point of Extensible was "clients have to accept payloads with extra fields in these places". But if that's always true, then it isn't adding anything.

I think what's actually needed is some constraints on how implementations can represent unilateral extensions of the protocol (which are hopefully rare, but for which there is precedent in e.g. capabilities). In that case we require that any extra fields have a : in the name, and commit to never specifying a field with such a name.

@jgraham
Copy link
Member

jgraham commented Jul 10, 2023

For the specific case of Message types, putting an explicit type field on seems like the obvious thing that we should have done (and still can do), i.e. type: "success", type: "error", and type: "event".

@jrandolf
Copy link
Contributor Author

jrandolf commented Jul 10, 2023

For the specific case of Message types, putting an explicit type field on seems like the obvious thing that we should have done (and still can do), i.e. type: "success", type: "error", and type: "event".

Perhaps we should try this then.

jrandolf added a commit that referenced this issue Jul 10, 2023
jrandolf added a commit that referenced this issue Jul 12, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants