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

Make httpResponseCode not worked with httpError #1105

Closed
david-perez opened this issue Feb 23, 2022 · 4 comments
Closed

Make httpResponseCode not worked with httpError #1105

david-perez opened this issue Feb 23, 2022 · 4 comments

Comments

@david-perez
Copy link
Contributor

david-perez commented Feb 23, 2022

The following Smithy model builds without any warnings:

$version: "1.0"

namespace com.amazonaws.simple

use aws.protocols#restJson1

@restJson1
@title("SimpleService")
service SimpleService {
    operations: [
        AnOperation,
    ],
}

@http(uri: "/operation", method: "GET")
operation AnOperation {
    input: AnOperationInput,
    output: AnOperationOutput,
    errors: [MyError]
}

structure AnOperationInput {
    @httpResponseCode
    responseCode: Integer
}

structure AnOperationOutput { }

@error("client")
@httpError(404)
structure MyError {
    @httpResponseCode
    responseCode: Integer
}

I'd expect it to be fail with errors:

  1. The spec says httpResponseCode is ignored if applied to operation input member. Why not enforce this by failing the build? It's clearly an error by the user.
  2. What response code should MyError set? Does the dynamic httpResponseCode one take precedence over the static httpError one if provided? I'd prefer it if we made httpResponseCode coupled with required, and outright rejected httpResponseCode applied to error structure members.
@mtdowling
Copy link
Member

The spec is clear here:

httpResponseCode is ignored when resolving the HTTP bindings of an operation's input structure.

There are various traits in Smithy that only have effect contextually, and forbidding them makes it so structures aren't reusable. Just ignore the trait if it isn't used as a top-level output member. We could only forbid this trait if the structure is marked as @input.

@david-perez david-perez changed the title Make httpResponseCode only work when applied to operation output member Make httpResponseCode not worked with httpError Feb 24, 2022
@david-perez
Copy link
Contributor Author

and forbidding them makes it so structures aren't reusable

Hadn't thought about this.

We could only forbid this trait if the structure is marked as @input.

It'd be a nice improvement.


I've amended the issue title in case you want to tackle the second item.

@mtdowling
Copy link
Member

That sounds fine with me and would only be a selector change in the prelude and spec change. Can you make that change?

syall pushed a commit to syall/smithy that referenced this issue Aug 26, 2022
- Changed the selector of `@httpResponseCode` to not select structures
  with the `@input` trait applied
- Updated the docs for `@httpResponseCode`
- Move `http-trait-conflicts` test to a `.smithy` file

Resolves smithy-lang#1105
@syall syall closed this as completed in 7d42189 Aug 26, 2022
@syall
Copy link
Contributor

syall commented Aug 26, 2022

The @httpResponseCode is no longer allowed to applied to @input structures, and the spec has been updated to clarify that @httpResponseCode will only affect HTTP bindings of output structures (#1359).

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

No branches or pull requests

3 participants