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

[wasip2] incoming http handler #8

Open
wants to merge 6 commits into
base: wasip2
Choose a base branch
from

Conversation

rajatjindal
Copy link
Contributor

This PR is first of many to support wasip2 in spin go sdk.

@rajatjindal
Copy link
Contributor Author

I'll update this branch to use wasm-tools-go from bytecodealliance org as that repo has been moved.

@rajatjindal
Copy link
Contributor Author

Hi @adamreese @michelleN

I believe this PR is ready for review. 🎉

thank you

@adamreese
Copy link
Member

Adding an extra line to the response output gives me this error

2024-09-20T18:07:05.502974Z ERROR spin_trigger_http::server: Error processing request: guest invocation failed

Caused by:
    0: error while executing at wasm backtrace:
           0: 0x42c98 - wit-component:shim!indirect-wasi:io/[email protected][method]output-stream.write
           1: 0x17a4b - main!interface:{Write:func:{slice:basic:uint8}{basic:int,named:error}}.Write$invoke
           2: 0x32eaf - main!fmt.Fprintln
           3: 0x34163 - main!wasi:http/[email protected]#handle
    1: write exceeded budget

@rajatjindal
Copy link
Contributor Author

Adding an extra line to the response output gives me this error

2024-09-20T18:07:05.502974Z ERROR spin_trigger_http::server: Error processing request: guest invocation failed

Caused by:
    0: error while executing at wasm backtrace:
           0: 0x42c98 - wit-component:shim!indirect-wasi:io/[email protected][method]output-stream.write
           1: 0x17a4b - main!interface:{Write:func:{slice:basic:uint8}{basic:int,named:error}}.Write$invoke
           2: 0x32eaf - main!fmt.Fprintln
           3: 0x34163 - main!wasi:http/[email protected]#handle
    1: write exceeded budget

Hi Adam, thanks for pointing this out. I have pushed a fix for it and it seems to be working now.

Copy link
Member

@adamreese adamreese left a comment

Choose a reason for hiding this comment

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

@rajatjindal This is a great start and I'm excited to have something to iterate on. To keep it consistent with the other Spin repos I feel like we should stick to GitHub Actions rather than introducing Dagger at this time. Could we break out Dagger to a separate conversation from this PR?

w.Header().Set("Content-Type", "text/plain")
w.Header().Set("foo", "bar")

//TODO(rajatjindal): calling WriteHeader is required right now, need to fix before merging
Copy link
Member

Choose a reason for hiding this comment

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

I think this TODO can be removed?

@rajatjindal
Copy link
Contributor Author

Could we break out Dagger to a separate conversation from this PR?

yes absolutely. I will push updated PR.

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.

2 participants