Wasmtime GitHub notifications bot (Aug 14 2023 at 23:09): 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 upstreamOn 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:
If this work has been discussed elsewhere, please include a link to that
conversation. If it was discussed in an issue, just mention "issue #...".Explain why this change is needed. If the details are in an issue already,
this can be brief.Our development process is documented in the Wasmtime book:
https://docs.wasmtime.dev/contributing-development-process.htmlPlease ensure all communication follows the code of conduct:
https://github.com/bytecodealliance/wasmtime/blob/main/CODE_OF_CONDUCT.md
-->
Wasmtime GitHub notifications bot (Aug 14 2023 at 23:09):pchickey requested jameysharp for a review on PR #6846.
Wasmtime GitHub notifications bot (Aug 14 2023 at 23:09):pchickey requested wasmtime-core-reviewers for a review on PR #6846.
Wasmtime GitHub notifications bot (Aug 14 2023 at 23:10):pchickey requested elliottt for a review on PR #6846.
Wasmtime GitHub notifications bot (Aug 14 2023 at 23:47):pchickey requested wasmtime-default-reviewers for a review on PR #6846.
Wasmtime GitHub notifications bot (Aug 14 2023 at 23:47):pchickey updated PR #6846.
Wasmtime GitHub notifications bot (Aug 15 2023 at 00:17):pchickey updated PR #6846.
Wasmtime GitHub notifications bot (Aug 15 2023 at 00:25):pchickey updated PR #6846.
Wasmtime GitHub notifications bot (Aug 15 2023 at 00:25):pchickey updated PR #6846.
Wasmtime GitHub notifications bot (Aug 15 2023 at 20:39):pchickey updated PR #6846.
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 theStreamRuntimeErrortype for runtime
errors, and all other errors trap.<!--
Please make sure you include the following information:
If this work has been discussed elsewhere, please include a link to that
conversation. If it was discussed in an issue, just mention "issue #...".Explain why this change is needed. If the details are in an issue already,
this can be brief.Our development process is documented in the Wasmtime book:
https://docs.wasmtime.dev/contributing-development-process.htmlPlease ensure all communication follows the code of conduct:
https://github.com/bytecodealliance/wasmtime/blob/main/CODE_OF_CONDUCT.md
-->
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_errorfunctionality
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)HostInputStreamand
HostOutputStreamtypes.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 theStreamRuntimeErrortype for runtime
errors, and all other errors trap.<!--
Please make sure you include the following information:
If this work has been discussed elsewhere, please include a link to that
conversation. If it was discussed in an issue, just mention "issue #...".Explain why this change is needed. If the details are in an issue already,
this can be brief.Our development process is documented in the Wasmtime book:
https://docs.wasmtime.dev/contributing-development-process.htmlPlease ensure all communication follows the code of conduct:
https://github.com/bytecodealliance/wasmtime/blob/main/CODE_OF_CONDUCT.md
-->
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_errorfunctionality 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)HostInputStreamandHostOutputStreamtypes.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
StreamRuntimeErrortype for runtime errors, and all other errors trap.<!--
Please make sure you include the following information:
If this work has been discussed elsewhere, please include a link to that
conversation. If it was discussed in an issue, just mention "issue #...".Explain why this change is needed. If the details are in an issue already,
this can be brief.Our development process is documented in the Wasmtime book:
https://docs.wasmtime.dev/contributing-development-process.htmlPlease ensure all communication follows the code of conduct:
https://github.com/bytecodealliance/wasmtime/blob/main/CODE_OF_CONDUCT.md
-->
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 ofResult:)
Wasmtime GitHub notifications bot (Aug 16 2023 at 22:16):pchickey updated PR #6846.
Wasmtime GitHub notifications bot (Aug 17 2023 at 21:37):pchickey merged PR #6846.
Last updated: Oct 23 2024 at 20:03 UTC