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

Semantics of constrained event streams for the server #1388

Open
david-perez opened this issue Sep 7, 2022 · 2 comments
Open

Semantics of constrained event streams for the server #1388

david-perez opened this issue Sep 7, 2022 · 2 comments
Labels
documentation This is a problem with documentation. guidance Question that needs advice or information.

Comments

@david-perez
Copy link
Contributor

What should a server do upon receiving an event message in an event stream that does not satisfy the modeled constraint traits?

For example, with the following model, say a client sends an event message with messageContent set to a string of length 1.

@streaming
union Event {
    regularMessage: EventStreamRegularMessage,
    errorMessage: EventStreamErrorMessage,
}

structure EventStreamRegularMessage {
    messageContent: LengthString
}

@error("server")
structure EventStreamErrorMessage {
    messageContent: LengthString
}

@length(min: 2, max: 69)
string LengthString

Should the server:

  • ignore the constraints and hand it to the application code?
  • swallow the message and not hand it to the application code?
  • terminate the event stream?
  • signal to the application code an invalid message was received? How?
@milesziemer milesziemer added the guidance Question that needs advice or information. label Sep 13, 2022
david-perez added a commit to smithy-lang/smithy-rs that referenced this issue Dec 7, 2022
Constrained shapes in the closure of an event stream are not supported
[0]. However, the `ValidateUnsupportedConstraints` validator was not
detecting constrained shapes in event stream errors because the
`EventStreamNormalizer` model transformer pulls them out of the
`@streaming` union shape.

This commit makes it so that the validator will detect such usages, by
leveraging the `SyntheticEventStreamUnionTrait` trait that gets attached
in the model transformer.

[0]: smithy-lang/smithy#1388
LukeMathWalker added a commit to smithy-lang/smithy-rs that referenced this issue Dec 9, 2022
Constrained shapes in the closure of an event stream are not supported
[0]. However, the `ValidateUnsupportedConstraints` validator was not
detecting constrained shapes in event stream errors because the
`EventStreamNormalizer` model transformer pulls them out of the
`@streaming` union shape.

This commit makes it so that the validator will detect such usages, by
leveraging the `SyntheticEventStreamUnionTrait` trait that gets attached
in the model transformer.

[0]: smithy-lang/smithy#1388

Co-authored-by: Luca Palmieri <20745048+LukeMathWalker@users.noreply.github.com>
@kstich kstich added the documentation This is a problem with documentation. label Aug 15, 2023
@ptrucks
Copy link

ptrucks commented Oct 16, 2023

My team just ran into this issue when trying to generate the Rust server SDK for a shared model. Interestingly, the Java and TypeScript code generators did not complain about the constraints applied to streamed events, but the Rust generator complained with a link to this issue.

Is there a reason to treat constraint errors for an event fundamentally different from deserialization errors for events?

Let's take for example:

operation GetMessages {
    input: GetMessageInput,
   output: GetMessageOutput,
}

@input
structure GetMessageInput {}

@output
structure GetMessageOutput {
   @httpPayload
   events: MessageEvents
}

@streaming
union MessageEvents {
    message: Message
}

structure Message {
    @length(min: 5)
    message: String
}

It seems intuitive to treat a message JSON which fails the @length constraint, e.g.:

{"message":"msg"}

in a similar way as

{"message":1337}

which also breaks the API contract because the type is number instead of string.

At the moment the second kind of check happens implicitly during deserialization, while the first kind is made impossible.

It is especially unfortunate that this also prevents enum types from appearing as event members, the removal of which affects generated client code.

@mtdowling
Copy link
Member

These should be validated, and the server should close the event stream. Ideally it sends back a Validation error just like with other normal operations, and if you don't have a modeled error of this type, send back a generic protocol error.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
documentation This is a problem with documentation. guidance Question that needs advice or information.
Projects
None yet
Development

No branches or pull requests

5 participants