Skip to content

Conversation

dicej
Copy link
Contributor

@dicej dicej commented Sep 15, 2025

This adds wasi:{cli,clocks,filesystem,http,random,sockets}@0.3.0-rc-2025-09-16 support, including inbound and outbound HTTP requests.

I've smoke tested this manually with a few WASIp3 components and plan to port over some or all of the existing WASIp2 tests.

Note that this builds on #3271

TODO:

  • Port (some) wasi:[email protected] tests
  • Add Instrument::in_current_span calls where necessary (e.g. in wasip3.rs)
  • Update to next release candidate once the release-37.0.0 branch supports it
  • Switch to the final 37.0.0 release once it's available

@dicej dicej requested review from alexcrichton and lann September 15, 2025 22:17
@dicej dicej force-pushed the p3 branch 2 times, most recently from 58ba9b5 to 4486786 Compare September 15, 2025 23:23
Copy link
Contributor

@alexcrichton alexcrichton left a comment

Choose a reason for hiding this comment

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

Looks good to me on all the Wasmtime bits, thanks! I'll defer to others for the Spin-related bits though.

@dicej dicej marked this pull request as ready for review September 29, 2025 17:33
@dicej
Copy link
Contributor Author

dicej commented Sep 29, 2025

I'm not able to repro the CI error on Mac or Linux. I tried rerunning the job, but it failed the second time. Not sure what's up. UPDATE: fixed

Copy link
Collaborator

@itowlson itowlson left a comment

Choose a reason for hiding this comment

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

Not sure my approval is worth much but here it is anyway! Great to see this, excited to give it a try!

>,
> + Send,
> {
// TODO: do we neeed to do anything with `fut`?
Copy link
Collaborator

Choose a reason for hiding this comment

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

Do we know the status of this TODO?

Copy link
Contributor Author

@dicej dicej Oct 2, 2025

Choose a reason for hiding this comment

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

Good question. It's also ignored in wasmtime-wasi, and it seems redundant to me. I assume it represents an error reading the request body, but that should be reported via the BoxBody<Bytes, ErrorCode> in the request; no need for a separate parameter. I'll ask Roman (who wrote that code) about it. Nevermind; see my comment below.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Oops, I should have read the docs more carefully. That future may be used to learn about errors the guest might encounter while doing whatever it decides to do with the response we return here. Arguably we don't care, though -- we've done our job producing the response, and we can't do anything about it here if the guest has trouble e.g. forwarding it elsewhere. I'll update the comment to say so.

Copy link
Collaborator

Choose a reason for hiding this comment

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

I don't quite understand, but probably don't need to, except to ask: would that include guest errors produced after the response has been finished? (The reason I ask is e.g. the Slack bot scenario, where the bulk of the work may happen after the ack response has been returned: current Spin can be a bit weird about errors in that phase if I remember rightly.)

(I realise "a bit weird about" is, uh, a bit vague, but I have forgotten the details.)

Copy link
Collaborator

Choose a reason for hiding this comment

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

Never mind, I'm an idiot: this is outbound HTTP. Sorry for the noise.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

would that include guest errors produced after the response has been finished?

The guest can send an error to fut at any time, i.e. before, during, or after the response has arrived in full and been delivered to the guest; there's nothing in the API that constraints when or why the guest would do that. In any case, there's not much we can do about it here 🤷 .

dicej added 12 commits October 2, 2025 15:54
v37.0.0 will be released later this month; this is a draft based on the Git
branch for that release so we're ready when the release happens.

Closes spinframework#3270

Signed-off-by: Joel Dice <[email protected]>
This is required by Wasmtime 37.

Signed-off-by: Joel Dice <[email protected]>
This adds wasi:{cli,clocks,filesystem,http,random,sockets}@0.3.0-rc-2025-08-15
support, including inbound and outbound HTTP requests.

I've smoke tested this manually with a few WASIp3 components and plan to port
over some or all of the existing WASIp2 tests.

TODO:
  - Port wasi:[email protected] tests
  - Add `Instrument::in_current_span` calls where necessary (e.g. in wasip3.rs)
  - Update to next release candidate once the `release-37.0.0` branch supports it
  - Switch to the final 37.0.0 release once it's available

Signed-off-by: Joel Dice <[email protected]>
This required adding the 0.3.0-rc-2025-08-15 WIT files to the tree so
`wit-bindgen` could use them.

I've also addressed Till's feedback to match only the exact 0.3.0 RC we support
when looking for the `wasi:http/handler` export.

Finally, this includes a temporary workaround for
bytecodealliance/wasmtime#11703, which was uncovered
by the streaming test added in this commit.

Signed-off-by: Joel Dice <[email protected]>
@dicej dicej enabled auto-merge (squash) October 2, 2025 22:37
Comment on lines +36 to +41
let Ok(name) = n.parse::<HeaderName>() else {
return None;
};
let Ok(value) = HeaderValue::from_bytes(v.as_bytes()) else {
return None;
};
Copy link
Collaborator

Choose a reason for hiding this comment

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

Just FYI

Suggested change
let Ok(name) = n.parse::<HeaderName>() else {
return None;
};
let Ok(value) = HeaderValue::from_bytes(v.as_bytes()) else {
return None;
};
let name = n.parse::<HeaderName>().ok()?;
let value = HeaderValue::from_bytes(v.as_bytes()).ok()?;

@dicej dicej merged commit 21268ac into spinframework:main Oct 3, 2025
59 of 65 checks passed
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.

6 participants