CRE-1613: Fix internal errors passed from WASM host to guest and add a standard test for it #1745
Conversation
…a standard test for it
|
👋 nolag, thanks for creating this pull request! To help reviewers, please consider creating future PRs as drafts first. This allows you to self-review and make any final changes before notifying the team. Once you're ready, you can mark it as "Ready for review" to request feedback. Thanks! |
There was a problem hiding this comment.
Pull request overview
This PR fixes the handling of internal errors passed from the WASM host to the guest by ensuring error indicators are properly negated when writing to WASM memory. A new standard test verifies that buffer overflow errors during capability responses are correctly propagated to the guest.
- Modified
truncateWasmWriteto return negated byte counts for error strings, properly signaling failures to the guest - Updated Go WASM guest code to handle negated error byte counts and include error messages
- Added a standard test that validates error propagation when response buffers are too small
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| pkg/workflows/wasm/host/standard_tests/host_wasm_write_errors_are_respected/main_wasip1.go | New test module that triggers a buffer overflow scenario by requesting capability output that exceeds available buffer space |
| pkg/workflows/wasm/host/standard_test.go | Adds test case verifying that oversized capability responses produce proper error messages |
| pkg/workflows/wasm/host/module.go | Updates truncateWasmWrite to negate the return value, signaling errors to the guest |
| pkg/workflows/wasm/host/internal/rawsdk/helpers_wasip1.go | Fixes error handling to extract error message from negated byte count and include it in the error |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
pkg/workflows/wasm/host/standard_tests/host_wasm_write_errors_are_respected/main_wasip1.go
Outdated
Show resolved
Hide resolved
✅ API Diff Results - No breaking changes |
| } | ||
|
|
||
| return write(memory, src, ptr, size) | ||
| // truncateWasmWrite is only called for returning error strings |
There was a problem hiding this comment.
Then maybe rename the function to make it more explicit?
4 participants
I added the test to both Go and TS. Go already passed, TS needed a small change in the Rust JS host, then passed.
I'll open the PRs to update the standard test and fix TS after this is merged because I need a tag.