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

operationShapesThatMustHaveValidationException does not account for event streaming members. #3813

Closed
drganjoo opened this issue Sep 2, 2024 · 1 comment · Fixed by #3814
Labels
bug Something isn't working server Rust server SDK

Comments

@drganjoo
Copy link
Contributor

drganjoo commented Sep 2, 2024

When an operation's input includes an event streaming member, the builder for the operation input or output can raise a ConstraintViolation if build_enforcing_all_constraints is called and the event streaming member field has not been set.

The From implementation that converts ConstraintViolation to RequestRejection references ValidationException. However, if the user has not added ValidationException to the errors list, a build-time error is not generated to prompt the user to include ValidationException.

The operationShapesThatMustHaveValidationException method should raise an error in this case.

Here is a sample model:

service ConstrainedService {
    operations: [EventStreamsOperation]
}

structure EventStreamsOperationInputOutput {
    @httpPayload
    events: Event,
}

@http(uri: "/event-streams-operation", method: "POST")
operation EventStreamsOperation {
    input: EventStreamsOperationInputOutput,
    output: EventStreamsOperationInputOutput,
}

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

structure EventStreamRegularMessage {
    messageContent: String
}

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

In the operation input builder, build_enforcing_all_constraints returns a ConstraintViolation if constraints are not met:

        fn build_enforcing_all_constraints(
            self,
        ) -> Result<crate::input::EventStreamsOperationInput, ConstraintViolation> {
            Ok(
                crate::input::EventStreamsOperationInput {
                    events: self
                        .events
                        .ok_or(ConstraintViolation::MissingEvents)?,
                },
            )
        }

The From implementation for converting ConstraintViolation to RequestRejection uses crate::error::ValidationException:

    impl ::std::convert::From<ConstraintViolation>
        for ::aws_smithy_http_server::protocol::rpc_v2_cbor::rejection::RequestRejection
    {
        fn from(constraint_violation: ConstraintViolation) -> Self {
            let first_validation_exception_field =
                constraint_violation.as_validation_exception_field("".to_owned());
            let validation_exception = crate::error::ValidationException {

The generated Rust code errors out with:

error[E0412]: cannot find type `ValidationExceptionField` in module `crate::model`
  --> src/input.rs:78:28
   |
78 |         ) -> crate::model::ValidationExceptionField {
   |                            ^^^^^^^^^^^^^^^^^^^^^^^^ not found in `crate::model`
@drganjoo drganjoo added the server Rust server SDK label Sep 2, 2024
@david-perez
Copy link
Contributor

You must always have ValidationException in an event streaming operation because the event (the member on the input structure that targets a Smithy union/ Rust enum) is always required.

@david-perez david-perez added the bug Something isn't working label Sep 3, 2024
@drganjoo drganjoo changed the title operationsThatMustHaveValidationException does not account for event streaming members. operationShapesThatMustHaveValidationException does not account for event streaming members. Sep 3, 2024
github-merge-queue bot pushed a commit that referenced this issue Sep 4, 2024
…eption` (#3814)

This PR addresses an issue where, if an operation's input includes an
event streaming member, the builder for the operation input or output
may raise a `ConstraintViolation` when `build_enforcing_all_constraints`
is called and the event streaming member field is not set. This occurs
because the member shape is required.

The standard error message that is shown when `ValidationException` is
not attached to an operation is also displayed in this case:

*Operation test#TestOperation takes in input that is constrained
(https://awslabs.github.io/smithy/2.0/spec/constraint-traits.html), and
as such can fail with a validation exception. You must model this
behavior in the operation shape in your model file.*

```smithy
use smithy.framework#ValidationException

operation TestOperation {
    ...
    errors: [..., ValidationException] // <-- Add this.
}
```
Closes: [3813](#3813)

---------

Co-authored-by: Fahad Zubair <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working server Rust server SDK
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants