Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat(0.3): rework stream error handling/remove wasi:io usage #143

Merged
merged 1 commit into from
Jan 28, 2025

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
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