Conversation
Signed-off-by: Ryan <r.ekhoff@clockworklabs.io>
gefjon
left a comment
There was a problem hiding this comment.
I'd still like review from @JasonAtClockwork , as he's more familiar with C# than I am, but this looks good to me. I think we should be at feature parity with Rust and TypeScript enough to add a module sdk-test-procedure-cs and to use it in the procedure_tests in sdks/rust/tests/test.rs. I'll create a ticket for that, and we can discuss how to prioritize it.
JasonAtClockwork
left a comment
There was a problem hiding this comment.
Looks good to me. I ran the tests with diving in a bit and added extra debugging to see the output which also looks great.
|
One comment I wanted to keep separate from the review, I think this officially means we can kill the /sdks/csharp/examples~/regression-tests/procedure-client. Should we make a separate issue for that? It's not an emergency but will speed up regression tests a bit. |
Great point! Created #3954 so we can track this. |
Description of Changes
Rust added procedure-scoped HTTP via the
procedure_http_requestABI (see Rust PR #3684) to C# host bindings.What changed:
BSATN.Runtime/HttpWireTypes.cs) to match exactly the Rust stable types:spacetimedb_lib::http::Requestfields and order:method,headers,timeout,uri,versionspacetimedb_lib::http::Responsefields and order:headers,version,codename: string, value: bytes); no additional metadata is encoded.ProcedureContext.Httpsupport (Runtime/Http.cs) that:HttpRequestWire+ sends body bytes viaprocedure_http_requestHttpResponseWireand returns(response, body)as a typedHttpResponseHTTP_ERROR, decodes the error payload as a BSATNstringWOULD_BLOCK_TRANSACTION, returns a stable error message (host rejects blocking HTTP while a mut tx is open)procedure_http_requestimport to the C# wasm shim (Runtime/bindings.c) under thespacetime_10.3import module.[BytesSource; 2]).Runtime/Internal/Module.call_procedure): it must either returnErrno.OKor trap (log + rethrow). This mirrors the host’s expectations and avoids “unexpected errno values from guest entrypoints” behavior.Behavioral notes vs Rust
expect(...)/panic on “should never happen” serialization failures; C# runtime explicitly avoids throwing across the procedure boundary for the HTTP client path and instead returnsResult.Errfor unexpected failures. This prevents whole-module traps from user-space HTTP usage while still surfacing rich errors.API and ABI breaking changes
No new host ABI introduced, and does not change the syscall signature.
Adds API:
ProcedureContextBase.Httpis available for proceduresNo existing public types/method signatures are removed
expect(...)s and may panic/trap on internal “should not happen” cases. On the other hand, C#'sHttpClient.Sendexplicitly converts unexpected failures intoResult.Errto avoid trapping the module. This is a behavioral difference, but not an API break.Expected complexity level and risk
2 - Mostly just creating equivalents to the Rust version.
Testing
C# regression tests updated to cover:
ReadMySchemaViaHttp)InvalidHttpRequest) returning error without trappingTesting performed:
Success.