Clarify http payload binding error message

This adds some more verbose clarification for when somebody binds a member to the http request body for a method where that binding may not be permitted in all http clients.
smithy-lang · Oct 26, 2021 · 4bdca6c · 4bdca6c
1 parent 4ebc634
commit 4bdca6c
Show file tree

Hide file tree

Showing 2 changed files with 26 additions and 13 deletions.
diff --git a/...java/software/amazon/smithy/model/validation/validators/HttpMethodSemanticsValidator.java b/...java/software/amazon/smithy/model/validation/validators/HttpMethodSemanticsValidator.java
@@ -116,16 +116,29 @@ private List<ValidationEvent> validateOperation(
 
         List<HttpBinding> payloadBindings = bindingIndex.getRequestBindings(shape, HttpBinding.Location.PAYLOAD);
         List<HttpBinding> documentBindings = bindingIndex.getRequestBindings(shape, HttpBinding.Location.DOCUMENT);
-        if (semantics.allowsRequestPayload != null
-                && !semantics.allowsRequestPayload
-                && (!payloadBindings.isEmpty() || !documentBindings.isEmpty())) {
-            // Detect location and combine to one list for messages
-            String document = payloadBindings.isEmpty() ? "document" : "payload";
-            payloadBindings.addAll(documentBindings);
-            events.add(danger(shape, trait, String.format(
-                    "This operation uses the `%s` method in the `http` trait, but "
-                    + "has the following members bound to the %s: %s", method, document,
-                    ValidationUtils.tickedList(payloadBindings.stream().map(HttpBinding::getMemberName)))));
+        if (semantics.allowsRequestPayload != null && !semantics.allowsRequestPayload) {
+            if (!payloadBindings.isEmpty()) {
+                events.add(danger(shape, trait, String.format(
+                        "This operation uses the `%s` method in the `http` trait, but the `%s` member is sent as the "
+                            + "payload of the request because it is marked with the `httpPayload` trait. Many HTTP "
+                            + "clients do not support payloads with %1$s requests. Consider binding this member to "
+                            + "other parts of the HTTP request such as a query string parameter using the `httpQuery` "
+                            + "trait, a header using the `httpHeader` trait, or a path segment using the `httpLabel` "
+                            + "trait.",
+                        method, payloadBindings.get(0).getMemberName()
+                )));
+            } else if (!documentBindings.isEmpty()) {
+                events.add(danger(shape, trait, String.format(
+                        "This operation uses the `%s` method in the `http` trait, but the following members "
+                            + "are sent as part of the payload of the request: %s. These members are sent as part "
+                            + "of the payload because they are not explicitly configured to be sent in headers, in the "
+                            + "query string, or in a URI segment. Many HTTP clients do not support payloads with %1$s "
+                            + "requests. Consider binding these members to other parts of the HTTP request such as "
+                            + "query string parameters using the `httpQuery` trait, headers using the `httpHeader` "
+                            + "trait, or URI segments using the `httpLabel` trait.",
+                        method, ValidationUtils.tickedList(documentBindings.stream().map(HttpBinding::getMemberName))
+                )));
+            }
         }
 
         return events;

diff --git a/...software/amazon/smithy/model/errorfiles/validators/http-method-semantics-validator.errors b/...software/amazon/smithy/model/errorfiles/validators/http-method-semantics-validator.errors
@@ -1,6 +1,6 @@
-[DANGER] ns.foo#K: This operation uses the `GET` method in the `http` trait, but has the following members bound to the payload: `payload` | HttpMethodSemantics
-[DANGER] ns.foo#L: This operation uses the `GET` method in the `http` trait, but has the following members bound to the document: `payload` | HttpMethodSemantics
-[DANGER] ns.foo#M: This operation uses the `DELETE` method in the `http` trait, but has the following members bound to the document: `payload` | HttpMethodSemantics
+[DANGER] ns.foo#K: This operation uses the `GET` method in the `http` trait, but the `payload` member is sent as the payload of the request because it is marked with the `httpPayload` trait. Many HTTP clients do not support payloads with GET requests. Consider binding this member to other parts of the HTTP request such as a query string parameter using the `httpQuery` trait, a header using the `httpHeader` trait, or a path segment using the `httpLabel` trait. | HttpMethodSemantics
+[DANGER] ns.foo#L: This operation uses the `GET` method in the `http` trait, but the following members are sent as part of the payload of the request: `payload`. These members are sent as part of the payload because they are not explicitly configured to be sent in headers, in the query string, or in a URI segment. Many HTTP clients do not support payloads with GET requests. Consider binding these members to other parts of the HTTP request such as query string parameters using the `httpQuery` trait, headers using the `httpHeader` trait, or URI segments using the `httpLabel` trait. | HttpMethodSemantics
+[DANGER] ns.foo#M: This operation uses the `DELETE` method in the `http` trait, but the following members are sent as part of the payload of the request: `payload`. These members are sent as part of the payload because they are not explicitly configured to be sent in headers, in the query string, or in a URI segment. Many HTTP clients do not support payloads with DELETE requests. Consider binding these members to other parts of the HTTP request such as query string parameters using the `httpQuery` trait, headers using the `httpHeader` trait, or URI segments using the `httpLabel` trait. | HttpMethodSemantics
 [WARNING] ns.foo#G: This operation uses the `POST` method in the `http` trait, but is marked with the readonly trait | HttpMethodSemantics
 [WARNING] ns.foo#H: This operation uses the `DELETE` method in the `http` trait, but is not marked with the idempotent trait | HttpMethodSemantics
 [WARNING] ns.foo#I: This operation uses the `GET` method in the `http` trait, but is not marked with the readonly trait | HttpMethodSemantics