Fill in the 'error' variant #52
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -3,7 +3,7 @@ | |
| /// their headers, trailers, and bodies. | ||
| interface types { | ||
| use wasi:clocks/monotonic-clock.{duration}; | ||
| use wasi:io/streams.{input-stream, output-stream, error as stream-error}; | ||
| use wasi:io/poll.{pollable}; | ||
|
|
||
| /// This type corresponds to HTTP standard Methods. | ||
|
|
@@ -27,16 +27,85 @@ interface types { | |
| other(string) | ||
| } | ||
|
|
||
| /// These cases are inspired by the IANA HTTP Proxy Error Types: | ||
| /// https://www.iana.org/assignments/http-proxy-status/http-proxy-status.xhtml#table-http-proxy-error-types | ||
| variant error-code { | ||
| DNS-timeout, | ||
| DNS-error(DNS-error-payload), | ||
| destination-not-found, | ||
| destination-unavailable, | ||
| destination-IP-prohibited, | ||
| destination-IP-unroutable, | ||
| connection-refused, | ||
| connection-terminated, | ||
| connection-timeout, | ||
| connection-read-timeout, | ||
| connection-write-timeout, | ||
| connection-limit-reached, | ||
| TLS-protocol-error, | ||
| TLS-certificate-error, | ||
| TLS-alert-received(TLS-alert-received-payload), | ||
| HTTP-request-denied, | ||
| HTTP-request-length-required, | ||
|
There was a problem hiding this comment. The generic There was a problem hiding this comment. My rationale laid out here is that the remaining 4xx space would best be covered by returning an Following the addition of the There was a problem hiding this comment. The two cases from wasmtime-wasi-http that would be nice to cover are 400 and 405. We don't have anything that maps nicely to 400 right now, which is what was useful about the original There was a problem hiding this comment. @elliottt what errors are you wanting to map to 400? There was a problem hiding this comment. When we receive an error from hyper that returns true only for There was a problem hiding this comment. I think that's the right choice. The There was a problem hiding this comment. This is the specific case I'm thinking of, where the outgoing-handler needs to reject a There was a problem hiding this comment. Gotcha, I don't think There was a problem hiding this comment.
But returning There was a problem hiding this comment. Right, the remaining 4xx codes are those that really can't be answered by the environment. If you notice a case we've missed, let's add it as new variants in a follow-up PR |
||
| HTTP-request-body-size(option<u64>), | ||
| HTTP-request-method-invalid, | ||
| HTTP-request-URI-invalid, | ||
| HTTP-request-URI-too-long, | ||
| HTTP-request-header-section-size(option<u32>), | ||
| HTTP-request-header-size(option<field-size-payload>), | ||
|
Comment on lines
+54
to
+55
There was a problem hiding this comment. We should also add: |
||
| HTTP-request-trailer-section-size(option<u32>), | ||
| HTTP-request-trailer-size(field-size-payload), | ||
| HTTP-response-incomplete, | ||
| HTTP-response-header-section-size(option<u32>), | ||
| HTTP-response-header-size(field-size-payload), | ||
| HTTP-response-body-size(option<u64>), | ||
| HTTP-response-trailer-section-size(option<u32>), | ||
| HTTP-response-trailer-size(field-size-payload), | ||
| HTTP-response-transfer-coding(option<string>), | ||
| HTTP-response-content-coding(option<string>), | ||
| HTTP-response-timeout, | ||
| HTTP-upgrade-failed, | ||
| HTTP-protocol-error, | ||
|
There was a problem hiding this comment.
There was a problem hiding this comment.
I worry that if we expose this extra debugging information to clients, this will become load-bearing like the situation we have with 503s and custom HTTP/1 reason phrases. Perhaps we could instead encourage embedders to treat this as a cause for internal logging. There was a problem hiding this comment. OK, we will not add this information now. We may find some way to provide more debugging or error context in the medium term, but not now. There was a problem hiding this comment. No, I think we really need to provide some There was a problem hiding this comment. @pchickey I think it comes down to who the "user" is that we want to empower to debug. If the user is a person who moves their code from one hosting provider to another and suddenly they start seeing protocol errors for requests to the same origin, that is very unlikely to be actionable by that user. It almost certainly indicates a problem with the embedder or the network between the embedder and the origin. In this situation, adding a free-form string at best gives that user something to paste into the support request with their new provider. If you have other user profiles in mind that would benefit from the debugging info directly, I don't want to hold up progress on the spec. I would just recommend that any embedder who wants to avoid ossification not populate this field for production traffic. There was a problem hiding this comment. @acfoltzer that is too narrow a definition of user. This spec already has implementations in progress that will be deployed beyond vendor-managed FaaS. For example the wasmtime-wasi-http implementation is intended to be run via There was a problem hiding this comment. We're designating |
||
| loop-detected, | ||
| configuration-error, | ||
| /// This is a catch-all error for anything that doesn't fit cleanly into a | ||
| /// more specific case. It also includes an optional string for an | ||
| /// unstructured description of the error. Users should not depend on the | ||
| /// string for diagnosing errors, as it's not required to be consistent | ||
| /// between implementations. | ||
| internal-error(option<string>) | ||
| } | ||
|
|
||
| /// Defines the case payload type for `DNS-error` above: | ||
| record DNS-error-payload { | ||
| rcode: option<string>, | ||
| info-code: option<u16> | ||
| } | ||
|
|
||
| /// Defines the case payload type for `TLS-alert-received` above: | ||
| record TLS-alert-received-payload { | ||
| alert-id: option<u8>, | ||
| alert-message: option<string> | ||
| } | ||
|
|
||
| /// Defines the case payload type for `HTTP-response-{header,trailer}-size` above: | ||
| record field-size-payload { | ||
| field-name: option<string>, | ||
| field-size: option<u32> | ||
| } | ||
|
|
||
| /// Attempts to extract a http-related `error` from the stream `error` | ||
| /// provided. | ||
| /// | ||
| /// Stream operations which return `stream-error::last-operation-failed` have | ||
| /// a payload with more information about the operation that failed. This | ||
| /// payload can be passed through to this function to see if there's | ||
| /// http-related information about the error to return. | ||
| /// | ||
| /// Note that this function is fallible because not all stream-related errors | ||
| /// are http-related errors. | ||
| http-error-code: func(err: borrow<stream-error>) -> option<error-code>; | ||
|
|
||
| /// This type enumerates the different kinds of errors that may occur when | ||
| /// setting or appending to a `fields` resource. | ||
| variant header-error { | ||
|
|
@@ -261,7 +330,7 @@ interface types { | |
| /// implementation determine how to respond with an HTTP error response. | ||
| set: static func( | ||
| param: response-outparam, | ||
| response: result<outgoing-response, error-code>, | ||
| ); | ||
| } | ||
|
|
||
|
|
@@ -336,7 +405,7 @@ interface types { | |
| /// as well as any trailers, were received successfully, or that an error | ||
| /// occured receiving them. The optional `trailers` indicates whether or not | ||
| /// trailers were present in the body. | ||
| get: func() -> option<result<option<trailers>, error-code>>; | ||
| } | ||
|
|
||
| /// Represents an outgoing HTTP Response. | ||
|
|
@@ -432,7 +501,7 @@ interface types { | |
| /// occured. Errors may also occur while consuming the response body, | ||
| /// but those will be reported by the `incoming-body` and its | ||
| /// `output-stream` child. | ||
| get: func() -> option<result<result<incoming-response, error-code>>>; | ||
|
|
||
| } | ||
| } | ||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The list below already diverged from the IANA table.