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

Allow the Zig SDK to return arbitrarily big responses #218

Open
ereslibre opened this issue Sep 14, 2023 · 3 comments · May be fixed by #264
Open

Allow the Zig SDK to return arbitrarily big responses #218

ereslibre opened this issue Sep 14, 2023 · 3 comments · May be fixed by #264
Labels
🐛 bug Something isn't working 👋 good first issue Good for newcomers 🛟 help wanted Extra attention is needed 🔨 sdks Issues related to language SDKs

Comments

@ereslibre
Copy link
Member

Describe the bug

At this time, the Zig SDK contains some hardcoded values when preparing the response object for Wasm Workers Server.

We should be able to return arbitrarily big responses from a Zig worker, and so, the Zig SDK should use allocation in order to prepare these responses, regardless of how big the response is.

Reproduction steps

  1. Create a Zig worker following the documentation
  2. Make it return a relatively big response
  3. See crash due to memory limits

Expected behavior

An answer is produced normally, and returned by Wasm Workers Server back to the user.

Additional context

No response

@ereslibre ereslibre added 🐛 bug Something isn't working 🛟 help wanted Extra attention is needed 🔨 sdks Issues related to language SDKs labels Sep 14, 2023
@Angelmmiguel Angelmmiguel added the 👋 good first issue Good for newcomers label Sep 14, 2023
@sea-grass
Copy link
Contributor

Hi there, great project! Is it okay if I take this on, and also make some additional updates to the Zig SDK and examples? In addition to solving this issue, I'd like to improve memory management (allow user to provide their own allocator, to perform cleanup after writing response to stdout, etc.), update the SDK and examples to use the Zig package manager (in Zig 0.12.0), and fix zig fmt issues in the SDK/example code as of Zig 0.12 (I think zig fmt was more lenient when the wws Zig SDK was initially written).

@voigt
Copy link
Contributor

voigt commented Feb 8, 2024

@sea-grass would be really happy to see this! Happy to help/review! :)

@sea-grass
Copy link
Contributor

Great, thanks @voigt. I'll get a PR going within the next few days and tag you as a reviewer so we can continue the discussion there!

sea-grass added a commit to sea-grass/wasm-workers-server that referenced this issue Feb 9, 2024
This commit introduces `kits/zig/wws`, with the intention of replacing `kits/zig/worker`.
The design of the new SDK improves upon the previous one by exposing utilities for parsing wws requests and serializing wws responses, to give the user more control over memory while still keeping the SDK easy to use.
The name was changed to make it more ergonomic within the Zig ecosystem (the developer will pull in a `wws` dependency to interface with wws, rather than a `worker` dependency).
@sea-grass sea-grass linked a pull request Feb 9, 2024 that will close this issue
9 tasks
sea-grass added a commit to sea-grass/wasm-workers-server that referenced this issue Feb 23, 2024
This commit introduces `kits/zig/wws`, with the intention of replacing `kits/zig/worker`.
The design of the new SDK improves upon the previous one by exposing utilities for parsing wws requests and serializing wws responses, to give the user more control over memory while still keeping the SDK easy to use.
The name was changed to make it more ergonomic within the Zig ecosystem (the developer will pull in a `wws` dependency to interface with wws, rather than a `worker` dependency).
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
🐛 bug Something isn't working 👋 good first issue Good for newcomers 🛟 help wanted Extra attention is needed 🔨 sdks Issues related to language SDKs
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants