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

Improve error when service is missing protocol #3551

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

djedward
Copy link
Contributor

@djedward djedward commented Apr 3, 2024

resolves #2452

By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.

Motivation and Context

Issue 2452

Description

This change updates the error message received when the model is missing a protocol or the protocol specified isn't found. The issue recommended adding some line breaks/wording changes. More than happy to adjust the formatting/wording as desired. This is what they look like now:

Service must have a protocol trait. Available protocols:
        aws.protocols#restJson1
        aws.protocols#restXml
        aws.protocols#awsJson1_0
        aws.protocols#awsJson1_1
Unable to find a matching protocol. Model specifies aws.protocols#awsJson1_0, but must match an available protocol:
        aws.protocols#restJson1
        aws.protocols#restXml
        aws.protocols#awsJson1_1

Testing

Added 3 new tests to cover success, missing protocol and no matching protocol

codegen-core:check
codegen-server:check
codegen-server-test:check

Checklist

  • I have updated CHANGELOG.next.toml if I made changes to the smithy-rs codegen or runtime crates
  • I have updated CHANGELOG.next.toml if I made changes to the AWS SDK, generated SDK code, or SDK runtime crates

By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.

@djedward djedward requested review from a team as code owners April 3, 2024 22:46
@drganjoo
Copy link
Contributor

drganjoo commented Apr 3, 2024

Is it possible to implement this as a Validator? For example, similar to how the range check is implemented. Using a Validator results in much nicer error output.

Thanks.

@djedward djedward force-pushed the main-issue2452 branch 2 times, most recently from 9343a39 to 9b3b33a Compare April 3, 2024 23:22
resolves smithy-lang#2452
----

_By submitting this pull request, I confirm that you can use, modify,
copy, and redistribute this contribution, under the terms of your
choice._
@djedward
Copy link
Contributor Author

djedward commented Apr 3, 2024

Sure, I can look into that.

@david-perez
Copy link
Contributor

An internal user also reported that the error was confusing because they didn't know what these protocols were about. We could link for each protocol its doc page under https://smithy.io/2.0/aws/protocols/index.html

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Improve error message when the service doesn't have a protocol trait applied
4 participants