Stream: git-wasmtime

Topic: wasmtime / PR #6846 stream wit definition eliminates stre...


view this post on Zulip Wasmtime GitHub notifications bot (Aug 14 2023 at 23:09):

pchickey opened PR #6846 from bytecodealliance:pch/no_more_stream_error to bytecodealliance:main:

In https://github.com/WebAssembly/wasi-io/pull/38 we got review feedback
to eliminate the stream-error in favor of the empty error in wit
result<a>.

This means we cant use trappable error anymore, and therefore leads to
all this other unsightly transformation of the streams trait definition
and all its call sites.

We'll fix the wasmtime-wit-bindgen macro to support this case better in
the future, but rn we gotta stay synchronized with upstream

On the upside this showed us that the host stream trait design doesnt
differentiate between a runtime and a trapping error, so lets fix that
next

<!--
Please make sure you include the following information:

Our development process is documented in the Wasmtime book:
https://docs.wasmtime.dev/contributing-development-process.html

Please ensure all communication follows the code of conduct:
https://github.com/bytecodealliance/wasmtime/blob/main/CODE_OF_CONDUCT.md
-->

view this post on Zulip Wasmtime GitHub notifications bot (Aug 14 2023 at 23:09):

pchickey requested jameysharp for a review on PR #6846.

view this post on Zulip Wasmtime GitHub notifications bot (Aug 14 2023 at 23:09):

pchickey requested wasmtime-core-reviewers for a review on PR #6846.

view this post on Zulip Wasmtime GitHub notifications bot (Aug 14 2023 at 23:10):

pchickey requested elliottt for a review on PR #6846.

view this post on Zulip Wasmtime GitHub notifications bot (Aug 14 2023 at 23:47):

pchickey requested wasmtime-default-reviewers for a review on PR #6846.

view this post on Zulip Wasmtime GitHub notifications bot (Aug 14 2023 at 23:47):

pchickey updated PR #6846.

view this post on Zulip Wasmtime GitHub notifications bot (Aug 15 2023 at 00:17):

pchickey updated PR #6846.

view this post on Zulip Wasmtime GitHub notifications bot (Aug 15 2023 at 00:25):

pchickey updated PR #6846.

view this post on Zulip Wasmtime GitHub notifications bot (Aug 15 2023 at 00:25):

pchickey updated PR #6846.

view this post on Zulip Wasmtime GitHub notifications bot (Aug 15 2023 at 20:39):

pchickey updated PR #6846.

view this post on Zulip Wasmtime GitHub notifications bot (Aug 15 2023 at 21:46):

pchickey edited PR #6846:

In https://github.com/WebAssembly/wasi-io/pull/38 we got review feedback
to eliminate the stream-error in favor of the empty error in wit
result<a>.

This means we cant use trappable error anymore, and therefore leads to
all this other unsightly transformation of the streams trait definition
and all its call sites.

We'll fix the wasmtime-wit-bindgen macro to support this case better in
the future.

As we made this change, it occurred to us that the host stream trait design doesn't
distinguish between an error to be returned via a stream method at runtime, and an
error that traps execution. We defined the StreamRuntimeError type for runtime
errors, and all other errors trap.

<!--
Please make sure you include the following information:

Our development process is documented in the Wasmtime book:
https://docs.wasmtime.dev/contributing-development-process.html

Please ensure all communication follows the code of conduct:
https://github.com/bytecodealliance/wasmtime/blob/main/CODE_OF_CONDUCT.md
-->

view this post on Zulip Wasmtime GitHub notifications bot (Aug 15 2023 at 21:48):

pchickey edited PR #6846:

In https://github.com/WebAssembly/wasi-io/pull/38 we got review feedback
to eliminate the stream-error in favor of the empty error in wit
result<a>.

This means we cant use wasmtime-wit-bindgen's trappable_error functionality
anymore, so the signature of the streams binding trait its call sites are much less
idiomatic than before. We'll fix the wasmtime-wit-bindgen macro to support this
case better in the future, but for now we can live with this code being a little ugly,
since most users will be using the (much nicer) HostInputStream and
HostOutputStream types.

As we made this change, it occurred to us that the host stream trait design doesn't
distinguish between an error to be returned via a stream method at runtime, and an
error that traps execution. We defined the StreamRuntimeError type for runtime
errors, and all other errors trap.

<!--
Please make sure you include the following information:

Our development process is documented in the Wasmtime book:
https://docs.wasmtime.dev/contributing-development-process.html

Please ensure all communication follows the code of conduct:
https://github.com/bytecodealliance/wasmtime/blob/main/CODE_OF_CONDUCT.md
-->

view this post on Zulip Wasmtime GitHub notifications bot (Aug 15 2023 at 21:49):

pchickey edited PR #6846:

In https://github.com/WebAssembly/wasi-io/pull/38 we got review feedback to eliminate the stream-error in favor of the empty error (wit result<a>). Although upstream PR 38 is not merged yet, we are downstreaming this interface change now, expecting it will merge upstream soon.

The wit change means we cant use wasmtime-wit-bindgen's trappable_error functionality anymore, so the signature of the streams binding trait its call sites are much less idiomatic than before. We'll fix the wasmtime-wit-bindgen macro to support this case better in the future, but for now we can live with this code being a little ugly, since most users will be using the (much nicer) HostInputStream and HostOutputStream types.

As we made this change, it occurred to us that the host stream trait design doesn't distinguish between an error to be returned via a stream method at runtime, and an error that traps execution. We defined the StreamRuntimeError type for runtime errors, and all other errors trap.

<!--
Please make sure you include the following information:

Our development process is documented in the Wasmtime book:
https://docs.wasmtime.dev/contributing-development-process.html

Please ensure all communication follows the code of conduct:
https://github.com/bytecodealliance/wasmtime/blob/main/CODE_OF_CONDUCT.md
-->

view this post on Zulip Wasmtime GitHub notifications bot (Aug 15 2023 at 23:15):

elliottt submitted PR review:

This looks great! I'm looking forward to the day that we can have custom impls for Try, as it would be great to remove the double-nesting of Result :)

view this post on Zulip Wasmtime GitHub notifications bot (Aug 16 2023 at 22:16):

pchickey updated PR #6846.

view this post on Zulip Wasmtime GitHub notifications bot (Aug 17 2023 at 21:37):

pchickey merged PR #6846.


Last updated: Dec 23 2024 at 12:05 UTC