Skip to content

Conversation

@gefjon
Copy link
Contributor

@gefjon gefjon commented Nov 18, 2025

Description of Changes

Closes #3517 .

With this PR, procedures (at least, those defined in Rust modules) can perform HTTP requests! This is performed through a new field on the ProcedureContext, http: HttpClient, which has a method send for sending an http::Request, as well as a convenience wrapper get.

Internally, these methods hit the procedure_http_request ABI call / host function, which uses reqwest to perform an HTTP request. The request is run with a user-configurable timeout which defaults and is clamped to 500 ms.
Rather than exposing the HTTP stream to modules, we download the entire response body immediately, within the same timeout.

I've added an example usage of get to module-test which performs a request against localhost:3000 to read its own schema/moduledef.

This PR also makes all procedure-related definitions in the Rust module bindings library #[cfg(feature = "unstable")], as per #3644 . The rename of the /v1/database/:name/procedure/:name route is not included in this PR, so this does not close #3644 .

Left as TODOs are:

  • Metrics for recording request and response size.
  • Improving performance by stashing a long-lived reqwest::Client someplace.
    Currently we build a new Client for each request.
  • Improving performance (possibly) by passing the request-future to the global tokio executor
    rather than running it on the single-threaded database executor.

API and ABI breaking changes

Adds new APIs, which are marked as unstable. Adds a new ABI, which is not unstable in any meaningful way (we can't really do that). Marks unreleased APIs as unstable. Does not affect any pre-existing already-released APIs or ABIs.

Expected complexity level and risk

3 or so: networking is scary, and even though we impose a timeout which prevents these connections from being truly long-lived, they're still potentially long-lived on the scale of Tokio futures. It's possible that running them on the database core is problematic in some way, and so what I've left as a performance TODO could actually be a concurrency-correctness issue.

Testing

  • Manually wrote and executed some procedures which make HTTP requests.
  • Added two automated tests to the sdk-test suite, procedure::http_ok and procedure::http_err, which make successful and failing requests respectively, then return its result. A client then makes some assertions about the result.

With this PR, procedures (at least, those defined in Rust modules) can perform HTTP requests!
This is performed through a new field on the `ProcedureContext`, `http: HttpClient`,
which has a method `send` for sending an `http::Request`, as well as a convenience wrapper `get`.

Internally, these methods hit the `procedure_http_request` ABI call / host function,
which uses reqwest to perform an HTTP request.
The request is run with a user-configurable timeout which defaults and is clamped to 500 ms.
Rather than exposing the HTTP stream to modules,
we download the entire response body immediately, within the same timeout.

I've added an example usage of `get` to `module-test` which performs a request against `localhost:3000`
to read its own schema/moduledef.

Not included in this commit, but intended to be within this PR, are:
- [ ] A test using the `sdk-test` framework.
- [ ] Expanded documentation.

Left as TODOs are:
- Metrics for recording request and response size.
- Improving performance by stashing a long-lived `reqwest::Client` someplace.
  Currently we build a new `Client` for each request.
- Improving performance (possibly) by passing the request-future to the global tokio executor
  rather than running it on the single-threaded database executor.
@coolreader18
Copy link
Collaborator

Phoebe and I were discussing HttpRequest - IMO it shouldn't have a NonExhaustive variant, and it probably shouldn't be an enum. These are internal interchange types only used for communicating across the abi boundary, and the spacetimedb_lib crate is explicitly marked as unstable API - if a user ends up storing it in their table, that's on them. Accommodating the possibility just adds uncertainty to our actual data model.

Furthermore, if we did want to change HttpRequest in the future, it'd probably just be best to introduce a new abi function altogther, since that's our existing mechanism for versioning. Under the enum scheme, if we added a new variant and someone built a new module and ran it against an old spacetimedb server, it would be a runtime error only when the http request is actually made. Also, no other types in spacetimedb_lib are structured like this, except for RawModuleDef, which is necessary because of the way that __describe_module__ is an export and so can't be versioned.

@gefjon gefjon enabled auto-merge November 20, 2025 19:20
@gefjon gefjon disabled auto-merge November 20, 2025 19:21
@JasonAtClockwork
Copy link
Contributor

Ran the Unreal tests locally it appears to be the flaky nature of Unreal in CI.
image

@coolreader18 coolreader18 added this pull request to the merge queue Nov 20, 2025
Merged via the queue into master with commit 7df8719 Nov 20, 2025
23 of 25 checks passed
github-merge-queue bot pushed a commit that referenced this pull request Nov 21, 2025
# Description of Changes

Follow up to #3684. Moves `Error` and `Timeout` out of lib, so that we
don't have to implement `SpacetimeType` for them, and then removes the
http dependency altogether, so that `lib` can be leaner. I also got rid
of the separate `HttpValue` type, since it only really exists to mirror
the `http` crate and typescript won't make use of it.

# Expected complexity level and risk

1

# Testing

n/a - just code movement.
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.

Procedures: Mark unstable for initial release Procedures: Add HTTP API

5 participants