[0.3.0-draft] Make DNS-error-payload just contain a string.

[sunfishcode]

Having DNS-error-payload record contain a string field for rcode while a u16 field for info-code is surprising. If appplications will be expected to know the IANA values of the u16 INFO-CODE value, it would make sense for them to know the u16 for the RCODE value too. But if they won't, then they likely don't have any use for either u16 value, and just want a simple string.

I think wasi-http could plausibly go either way: Provide u16 values for both rcode and info-code, or neither, and instead just provide a string.

In this PR, I propose to just provide a string. My guess is that most applications in scope here don't need precise DNS error code information and basically just need a way to report that "it was DNS". And, not all host resolver libraries provide error information that includes RCODE and INFO-CODE, for example Rust's ToSocketAddrs trait or POSIX getaddrinfo.

Fixed #184.

[lukewagner]

I don't really have much understanding of this aspect of HTTP/DNS but, if the cases are explicitly enumerated by a standard document, could we instead replace the u16 with an enum? My impression is that the place for helpful-error-strings is the to-be-added error-context where there is explicit nondeterministic room for hosts to do whatever they want.

[pchickey]

I agree with Luke, if we can possibly use an enum to describe dns error cases we should prefer that to a string. And, along the same lines as my prior comment, it would be great if that same dns error enum can be used in wasi-sockets (which means wasi-http would take on a dep of wasi-sockets for that enum, which is fine with me).

Both of the (closed-source) wasi-http embeddings I am working on have structured information for what sort of dns error occurred, and applications that make use of that error code. Embeddings without structured information can make a best effort to map their particular case to a code, and fall back on the internal error case with a string if that is the best they can do.

[sunfishcode]

Either way works for me. I've now filed #206 with enums for rcode and info-code using values from IANA.

[sunfishcode]

Also, I didn't attempt to unify #206 with wasi-sockets yet. I'm not sure I'm ok with wasi-http taking a dependency on wasi-sockets.

[lann]

It does seem a bit odd to be exposing RCODEs in wasi:http. On the application side I'd typically expect to do my own resolution if I wanted that kind of detail or for any other "advanced" use of DNS like getting the destination from a SRV record.

We could instead enable consumers to do their own resolution, perhaps something like a request-options.override-connect-addr(some(ip-address)) method.

Coincidentally, this would have saved me a bit of time recently. 🙂

[pchickey]

wasi-sockets is already de-facto a "dependency" of wasi-http despite never appearing in the source text, because it is a dependency of wasi-cli, which is used as part of the worlds.

Rather than being part of the request options, I believe that the wasi:sockets/ip-name-lookup interface should be part of the proxy/service world, and also thats where the rcode enum should live.

[lukewagner]

I do see sockets in the deps directory of wasi-http, but I think that's more of an implementation detail of the wit-deps tool since there are no actual use of the wasi:sockets package anywhere in wasi-http and if you manually deleted the sockets directory, everything wasi-http would resolve just fine. Starting a new wasi:dns package that could contain more DNS definitions over time seems better aligned with how the separate standards are defined and, perhaps if/when we move to a monorepo, wouldn't impose much extra engineering burden?

[pchickey]

It will not resolve and it is not an artifact of wit-deps. As I described, is a transitive dependency via wasi-cli.

[p.hickey@KVKD0WG7VF:~/src]% git clone https://github.com/webassembly/wasi-http
Cloning into 'wasi-http'...
remote: Enumerating objects: 1643, done.
remote: Counting objects: 100% (320/320), done.
remote: Compressing objects: 100% (161/161), done.
remote: Total 1643 (delta 195), reused 174 (delta 159), pack-reused 1323 (from 2)
Receiving objects: 100% (1643/1643), 511.03 KiB | 10.22 MiB/s, done.
Resolving deltas: 100% (901/901), done.
[p.hickey@KVKD0WG7VF:~/src]% cd wasi-http
[p.hickey@KVKD0WG7VF:src/wasi-http]% rm -rf wit/deps/sockets
wit/deps/sockets/instance-network.wit
wit/deps/sockets/world.wit
wit/deps/sockets/ip-name-lookup.wit
wit/deps/sockets/tcp-create-socket.wit
wit/deps/sockets/udp.wit
wit/deps/sockets/tcp.wit
wit/deps/sockets/udp-create-socket.wit
wit/deps/sockets/network.wit
wit/deps/sockets
[p.hickey@KVKD0WG7VF:src/wasi-http]% wit-bindgen markdown wit
Error: failed to resolve directory while parsing WIT for path [wit]

Caused by:
    package not found
         --> wit/deps/cli/imports.wit:10:11
          |
       10 |   include wasi:sockets/imports@0.2.8;
          |           ^-----------

[lukewagner]

Ok, I suppose it's also a detail of wit-bindgen too. In particular, the wasi:cli/imports world is not used by wasi-http.

[lann]

It seems like there's a general problem here with how to deal with inevitable leaks in the HTTP abstraction. In addition to DNS-error there's also TLS-alert-received with its associated payload.

There are also common leaky features that would benefit from standardization like retrieving peer socket addresses or TLS session details (ref #4).