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

finish-{in|out}going-stream should each return a (pseudo-)future which resolves to a result #37

Closed
dicej opened this issue Jun 8, 2023 · 2 comments

Comments

@dicej
Copy link
Collaborator

dicej commented Jun 8, 2023

We're wrapping up our implementation of wasi-http for Spin, and noticed a few issues, including this one. Given that finish-outgoing-stream takes an option<trailers> and finish-incoming-stream returns the same, both of them need to do I/O, which should happen asynchronously and be fallible like all other I/O in the proposal.

In the case of finish-incoming-stream, we can work around the issue by reading the trailers implicitly as part of the last streams::read call on the body and then keep it around until finish-incoming-stream is called. That's what our implementation currently does, but there are drawbacks:

In the case of finish-outgoing-stream, we can again work around the issue by spawning a task that writes the trailers, but again we have no obvious way of reporting any errors that occur, and the application has no way of knowing when the trailers have actually made it to the network, successfully or otherwise.

Alternatively, we could declare that both calls block, but that would be odd considering everything else is asynchronous, and we're still left with the question of how to report I/O errors.

I realize that this whole issue may end up being irrelevant once we have real streams, but that's a long way away from what I understand.

Thoughts?

@lukewagner
Copy link
Member

Agreed! Great catch, that is indeed an oversight. I'll make a PR adding this in a bit.

@lukewagner
Copy link
Member

I took a stab at fixing this in #37, PTAL.

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

No branches or pull requests

2 participants