Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

fix http.Headers to outboundHeaders #1925

Merged
merged 1 commit into from
Oct 26, 2023

Conversation

rajatjindal
Copy link
Collaborator

while working on a tool that use GitHub golang sdk, I ran into some issues:

2023-10-23T12:07:49.350766Z ERROR spin_trigger_http: Error processing request: error while executing at wasm backtrace:
    0: 0x222a8c - wit-component:shim!indirect-fermyon:spin/http-send-request
    1: 0x21d091 - wit-component:adapter:wasi_snapshot_preview1!wasi_snapshot_preview1::State::with::he9630df140b841e6
    2: 0x21deba - wit-component:adapter:wasi_snapshot_preview1!wasi-outbound-http:request
    3: 0x222b2c - wit-component:shim!adapt-wasi_snapshot_preview1-wasi-outbound-http:request
    4: 0x178136 - <unknown>!interface:{RoundTrip:func:{pointer:named:net/http.Request}{pointer:named:net/http.Response,named:error}}.RoundTrip$invoke
    5: 0x1775f8 - <unknown>!interface:{RoundTrip:func:{pointer:named:net/http.Request}{pointer:named:net/http.Response,named:error}}.RoundTrip$invoke
    6: 0x1952a9 - <unknown>!spin_http_handle_http_request
    7: 0x1b7890 - <unknown>!__wasm_export_spin_http_handle_http_request
    8: 0x21dde5 - wit-component:adapter:wasi_snapshot_preview1!fermyon:spin/inbound-http#handle-request

Caused by:
    invalid utf-8 sequence of 1 bytes from index 1 

code:

package main

import (
	"context"
	"net/http"

	spinhttp "github.com/fermyon/spin/sdk/go/http"
	"github.com/google/go-github/v50/github"
	"golang.org/x/oauth2"
)

func init() {
	spinhttp.Handle(func(w http.ResponseWriter, r *http.Request) {
		ts := oauth2.StaticTokenSource(&oauth2.Token{AccessToken: "a valid gh token"})

		spinclient := spinhttp.NewClient()
		tc := oauth2.NewClient(context.WithValue(r.Context(), oauth2.HTTPClient, spinclient), ts)
		client := github.NewClient(tc)

		_, _, err := client.Git.GetRef(r.Context(), "rajatjindal", "goodfirstissue", "heads/main")
		if err != nil {
			http.Error(w, err.Error(), http.StatusInternalServerError)
			return
		}
	})
}

func main() {}

with the proposed fix (which is essentially a copy from internals.go:toSpinHeaders function), it seems to work.

cc @adamreese does this fix make sense?

Signed-off-by: Rajat Jindal <rajatjindal83@gmail.com>
@rajatjindal rajatjindal requested a review from adamreese October 23, 2023 12:13
@adamreese
Copy link
Member

Is this related to #1928?

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.

This looks correct to me. Definitely more readable

@adamreese
Copy link
Member

Wait, I'm still getting the error on TinyGo 0.30.0 without adding the -tags=purego

@rajatjindal
Copy link
Collaborator Author

Hi @adamreese , this PR is NOT related to #1928. This is a separate issue I found it to be happening only when I was using GitHub SDK for some reason.

@rajatjindal
Copy link
Collaborator Author

Hi @adamreese , this PR is NOT related to #1928. This is a separate issue I found it to be happening only when I was using GitHub SDK for some reason.

Hi @adamreese, given above explaination, are the changes still ok to merge?

@adamreese
Copy link
Member

LGTM

@rajatjindal rajatjindal merged commit da166ac into fermyon:main Oct 26, 2023
9 checks passed
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants