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

Fix buffer-len handling in the component adapter #381

Merged
merged 3 commits into from
Jun 11, 2024

Conversation

elliottt
Copy link
Contributor

Fix handling of the buffer-length errors in the component adapter by turning the compute.wit error type into a variant. This allows us to pass out the length that would have allowed the operation to succeed when raising the Error::BufferLen error.

This PR also adds the necessary plumbing for running additional tests as adapted components, which should help catch more of these bugs as more of the test suite is enabled for adapted components.

One change was required to the response test for it to work with the component model: testing that too-long header names returned an error required that we switch to explicitly constructing those header values, instead of passing an invalid length. This is due to the requirement that we be able to construct a valid string from the buffer/length arguments, and would trap when using the component adapter otherwise.

@elliottt elliottt force-pushed the trevor/component-testing branch from 9dd8d53 to 81f9d2e Compare June 11, 2024 20:31
@elliottt elliottt marked this pull request as ready for review June 11, 2024 20:31
@elliottt elliottt requested review from acw, acfoltzer and athomason June 11, 2024 20:42
Copy link
Contributor

@acfoltzer acfoltzer left a comment

Choose a reason for hiding this comment

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

Ah, yeah, it makes sense that this would need to change. The convention of using nwritten_out to communicate this value is... very much not Componenty. This looks good

@elliottt elliottt force-pushed the trevor/component-testing branch from 81f9d2e to 9a034f7 Compare June 11, 2024 23:07
@elliottt elliottt merged commit a1f1332 into main Jun 11, 2024
7 checks passed
@elliottt elliottt deleted the trevor/component-testing branch June 11, 2024 23:39
GeeWee pushed a commit to GeeWee/Viceroy that referenced this pull request Jul 25, 2024
* Add a single test that uses the component adapter

* Fix the adapter's handling of BufferLen errors

* Fix the response test to not use invalid buffers for long headers
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