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

OutgoingBody finish content length errors #83

Open
guybedford opened this issue Nov 23, 2023 · 4 comments
Open

OutgoingBody finish content length errors #83

guybedford opened this issue Nov 23, 2023 · 4 comments

Comments

@guybedford
Copy link
Contributor

guybedford commented Nov 23, 2023

When the written body is less than the content length, the only error code that fits is internal-error.

But when the written body is more than the content length, perhaps it would make sense to use the existing HTTP-request-body-size(option<u64>) error code which is meant to be for when the body is too long? Alternatively should this also be internal-error?

Then, for both of these, it would be nice to have a proper error code instead of making it an internal error. Perhaps a dedicated HTTP-request-body-content-length-invalid(option<u64>)?

@pchickey
Copy link
Contributor

When the written size is less than the content length, HTTP-{request,response}-body-size(option<u64>) is the correct variant to return throw from outgoing-body.finish. The outgoing-body will have to return the correct request/response variant of the error there based on what it was created from.

We need to add documentation to the error-code cases, and this information should be there, and made more explicit in the docs for outgoing-body.finish.

@guybedford
Copy link
Contributor Author

When the written size is less than the content length, HTTP-{request,response}-body-size(option) is the correct variant to return throw from outgoing-body.finish.

Note this is not the case for Wasmtime currently - which throws an internal-error here instead.

@pchickey
Copy link
Contributor

Thanks - @elliottt can you work on fixing that?

@guybedford
Copy link
Contributor Author

This was landed in Wasmtime in bytecodealliance/wasmtime#7591, so is only pending as a documentation update at this point that the error has been extended to apply to sizes less than the expected content length.

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