Skip to content

Conversation

@rvolosatovs
Copy link
Contributor

Remove wasi:io usage in wit-0.3.0-draft

Followed the same stream error handling convention as currently in

@rvolosatovs rvolosatovs changed the title feat(0.3): rework stream error handling feat(0.3): rework stream error handling/remove wasi:io usage Jan 17, 2025
@ricochet ricochet requested a review from lukewagner January 22, 2025 14:43
Copy link
Member

@lukewagner lukewagner left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The change to the constructor/new is actually pretty interesting, because it demonstrates how the "return a future<result<_, error-code>>" trick is necessary not just for functions returning a stream, but also functions taking a stream, and thus even the stream<T,SuccessT,ErrorT> extension (presented here) wouldn't eliminate it. This raises some interesting questions on whether the ErrorT should be able to flow both ways... but that's all for later, and I think what you're doing here is about the only possible thing we can do in a 0.3.0 timeframe to not regress from 0.2, which provided error-codes upon write failures. Nice work! (cc @dicej )

@rvolosatovs
Copy link
Contributor Author

@lukewagner I've rebased on latest main to pull in the change from #142 and resolve a merge conflict

Copy link
Member

@lukewagner lukewagner left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Given all the things in flight, I wasn't sure when's a good point to merge (and iterate in later PRs), but seems fine to merge now?

@rvolosatovs
Copy link
Contributor Author

Given all the things in flight, I wasn't sure when's a good point to merge (and iterate in later PRs), but seems fine to merge now?

I think it's fine to merge now indeed

@lukewagner lukewagner merged commit 19964e5 into WebAssembly:main Jan 28, 2025
1 check passed
@rvolosatovs rvolosatovs deleted the chore/remove-wasi-io branch March 14, 2025 15:05
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants