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

HTTP 500s sent via gRPC are now sent as gRPC errors, with the body as a tailer. #36

Merged
merged 6 commits into from
May 26, 2017

Conversation

tomwilkie
Copy link
Contributor

@tomwilkie tomwilkie commented May 25, 2017

Also, don't need to connect on a background goroutine any more, this is fixed in grpc/grpc-go#976.

Fixes #35

@tomwilkie tomwilkie force-pushed the 35-dont-mask-errors branch from f0803ed to 59994fa Compare May 25, 2017 12:44
})
}

func responseFromError(err error) (*HTTPResponse, bool) {

This comment was marked as abuse.

@@ -161,6 +164,40 @@ func (c *Client) ServeHTTP(w http.ResponseWriter, r *http.Request) {
}
}

func errorFromResponse(resp *HTTPResponse) error {

This comment was marked as abuse.

@tomwilkie tomwilkie force-pushed the 35-dont-mask-errors branch 2 times, most recently from c410cf0 to 958cb82 Compare May 26, 2017 10:07
@tomwilkie tomwilkie force-pushed the 35-dont-mask-errors branch from 7e16297 to 1cb91d9 Compare May 26, 2017 11:24
@tomwilkie tomwilkie merged commit 831aa99 into master May 26, 2017
@tomwilkie tomwilkie deleted the 35-dont-mask-errors branch May 26, 2017 11:57
@bboreham
Copy link
Collaborator

There's a note in circle.yml saying not to change it until this PR is "fixed"; can anyone shed light on what that would mean, and if it has happened?

@jml
Copy link
Contributor

jml commented Nov 21, 2017

No, I don't know, and regret not insisting on more context when I reviewed this.

At a guess, it's related to the CircleCI failure we can see in the commit log:

httpgrpc/httpgrpc.go:22:2: cannot find package "google.golang.org/genproto/googleapis/rpc/status" in any of:

	/go/src/github.com/weaveworks/common/vendor/google.golang.org/genproto/googleapis/rpc/status (vendor tree)

	/usr/local/go/src/google.golang.org/genproto/googleapis/rpc/status (from $GOROOT)

	/go/src/google.golang.org/genproto/googleapis/rpc/status (from $GOPATH)

httpgrpc/httpgrpc.go:24:2: cannot find package "google.golang.org/grpc/status" in any of:

	/go/src/github.com/weaveworks/common/vendor/google.golang.org/grpc/status (vendor tree)

	/usr/local/go/src/google.golang.org/grpc/status (from $GOROOT)

	/go/src/google.golang.org/grpc/status (from $GOPATH)

I would suggest removing the workaround (i.e. installing dep in the normal way, whatever that is), and seeing what happens.

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.

HTTPoGRPC masking errors
3 participants