Stream: git-wasmtime

Topic: wasmtime / PR #8026 Show true error reason in `wasmtime s...

Wasmtime GitHub notifications bot (Feb 28 2024 at 23:52):

abrown opened PR #8026 from abrown:serve-error-propagation to bytecodealliance:main:

The wasmtime serve command prints failed HTTP handling to stderr but, when the reason was due to failure inside the HTTP handler itself, the printed error message is not descriptive enough. After looking at long dumps of guest never invoked response-outparam::set method, I realized that errors inside the Tokio task are never propagated outwards. This change captures those errors and appends them to the printed error message.

<!--
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.html

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

Wasmtime GitHub notifications bot (Feb 28 2024 at 23:52):

abrown requested fitzgen for a review on PR #8026.

Wasmtime GitHub notifications bot (Feb 28 2024 at 23:52):

abrown requested wasmtime-core-reviewers for a review on PR #8026.

Wasmtime GitHub notifications bot (Feb 28 2024 at 23:53):

abrown created PR review comment:

@elliottt, I guess if we adopt this PR we can remove this TODO line?

Wasmtime GitHub notifications bot (Feb 28 2024 at 23:53):

abrown submitted PR review.

Wasmtime GitHub notifications bot (Feb 29 2024 at 00:14):

pchickey requested pchickey for a review on PR #8026.

Wasmtime GitHub notifications bot (Feb 29 2024 at 00:15):

pchickey submitted PR review.

Wasmtime GitHub notifications bot (Feb 29 2024 at 00:15):

pchickey created PR review comment:

Yes, please remove this TODO and we can land this PR

Wasmtime GitHub notifications bot (Feb 29 2024 at 00:15):

pchickey submitted PR review:

Thanks for this fix!

Wasmtime GitHub notifications bot (Feb 29 2024 at 00:51):

elliottt submitted PR review:

Looks great to me, thanks for making this change @abrown!

Wasmtime GitHub notifications bot (Feb 29 2024 at 00:51):

elliottt submitted PR review:

Looks great to me, thanks for making this change @abrown!

Wasmtime GitHub notifications bot (Feb 29 2024 at 00:51):

elliottt created PR review comment:

            // If the receiver has an error (`RecvError`), it only indicates that the task
            // exited before the sender was used, that the sender was dropped
            // before a response was sent. Instead we retrieve and

Wasmtime GitHub notifications bot (Feb 29 2024 at 00:51):

elliottt created PR review comment:

Do we need to bound the amount of time spent here, or are we guaranteed that the result should be available immediately because the receiver.await failed?

Wasmtime GitHub notifications bot (Mar 01 2024 at 18:07):

abrown updated PR #8026.

Wasmtime GitHub notifications bot (Mar 01 2024 at 18:11):

abrown updated PR #8026.

Wasmtime GitHub notifications bot (Mar 01 2024 at 18:13):

abrown submitted PR review.

Wasmtime GitHub notifications bot (Mar 01 2024 at 18:13):

abrown created PR review comment:

I kind of merged our two versions into 5a958a5; hopefully that still captures what you meant in this suggestion!

Wasmtime GitHub notifications bot (Mar 01 2024 at 18:20):

abrown created PR review comment:

As I looked at it, I concluded that yes, if the receiver failed it is because the task failed. But maybe I missed something or perhaps future changes will invalidate that. But in that case, wasmtime serve will be dumping errors to stderr anyways so it should be apparent to users that something is wrong apart from the (hypothetical) additional slowdown. And they'll probably care more about whatever caused the task to fail.

Wasmtime GitHub notifications bot (Mar 01 2024 at 18:20):

abrown submitted PR review.

Wasmtime GitHub notifications bot (Mar 01 2024 at 19:15):

abrown updated PR #8026.

Wasmtime GitHub notifications bot (Mar 01 2024 at 19:15):

abrown submitted PR review.

Wasmtime GitHub notifications bot (Mar 01 2024 at 19:15):

abrown created PR review comment:

Ok, added a comment about this assumption: f09760e

Wasmtime GitHub notifications bot (Mar 01 2024 at 19:15):

abrown updated PR #8026.

Wasmtime GitHub notifications bot (Mar 01 2024 at 19:16):

abrown edited PR review comment.

Wasmtime GitHub notifications bot (Mar 01 2024 at 19:16):

abrown has enabled auto merge for PR #8026.

Wasmtime GitHub notifications bot (Mar 01 2024 at 20:45):

alexcrichton merged PR #8026.

Last updated: Nov 22 2024 at 17:03 UTC