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

Support streaming google.api.HttpBody message #5

Merged
merged 4 commits into from
Nov 16, 2023

Conversation

devholic
Copy link

This PR introduces support for streaming the google.api.HttpBody message, which allows client to download large-sized payload.

@devholic devholic requested review from alexmt and aborilov November 10, 2023 08:18
Signed-off-by: Sunghoon Kang <[email protected]>
Copy link

@aborilov aborilov left a comment

Choose a reason for hiding this comment

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

Tested!
thanks!

Copy link

@aborilov aborilov left a comment

Choose a reason for hiding this comment

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

I think there is a problem with error casting.

{"level":"ERROR","ts":"2023-11-13T15:17:45Z","caller":"logging/logging.go:61","msg":"finished streaming call with code Internal","grpc.start_time":"2023-11-13T23:17:45+08:00","system":"grpc","span.kind":"server","grpc.service":"akuity.argocd.v1.ArgoCDService","grpc.method":"GetInstanceClusterManifests","actor.type":"user","actor.id":"123","grpc.code":"Internal","grpc.time_ms":13.889,"error":"rpc error: code = Internal desc = rpc error: code = Unavailable desc = unavailable","error":"rpc error: code = Internal desc = rpc error: code = Unavailable desc = unavailable","stacktrace":"github.com/akuityio/akuity-platform/internal/utils/grpc/logging.StreamServerInterceptor.func1\n\t/Users/aborilov/work/akuity/akuity-platform/internal/utils/grpc/logging/logging.go:61\ngoogle.golang.org/grpc.NewServer.chainStreamServerInterceptors.chainStreamInterceptors.func2\n\t/Users/aborilov/go/pkg/mod/google.golang.org/[email protected]/server.go:1480\ngoogle.golang.org/grpc.(*Server).processStreamingRPC\n\t/Users/aborilov/go/pkg/mod/google.golang.org/[email protected]/server.go:1644\ngoogle.golang.org/grpc.(*Server).handleStream\n\t/Users/aborilov/go/pkg/mod/google.golang.org/[email protected]/server.go:1741\ngoogle.golang.org/grpc.(*Server).serveStreams.func1.1\n\t/Users/aborilov/go/pkg/mod/google.golang.org/[email protected]/server.go:986"}

that's what server prints
and here what clients get in the body

{
  "error": {
    "code": 13,
    "message": "rpc error: code = Unavailable desc = unavailable",
    "details": []
  }
}

if err != nil {
return nil, nil, fmt.Errorf("send request: %w", err)
}
if res.IsError() {

Choose a reason for hiding this comment

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

because we are don't parse response res.Error will be empty, we need to do that same as what we do in DoStreamingRequest

	if rawRes.IsError() {
		body := rawRes.RawBody()
		defer func() { _ = body.Close() }()
		data, err := io.ReadAll(body)
		if err != nil {
			return nil, nil, fmt.Errorf("read error response body: %w", err)
		}

		var res streamingResponse
		if err := json.Unmarshal(data, &res); err != nil {
			return nil, nil, fmt.Errorf("unmarshal raw response: %w", err)
		}
		rawErrRes, ok := res[streamingResponseErrorKey]
		if !ok {
			return nil, nil, errors.New(string(data))
		}
		var errResp rpcstatus.Status
		if err := c.Unmarshal(rawErrRes, &errResp); err != nil {
			return nil, nil, fmt.Errorf("unmarshal error response: %w", err)
		}
		if err := status.ErrorProto(&errResp); err != nil {
			return nil, nil, err
		}
		return nil, nil, status.Error(HTTPStatusToCode(rawRes.StatusCode()), rawRes.String())
	}

@devholic devholic requested a review from aborilov November 16, 2023 01:50
@aborilov
Copy link

@devholic devholic merged commit 80c4013 into main Nov 16, 2023
2 checks passed
@devholic devholic deleted the hoon/streaming-httpbody branch November 16, 2023 13:49
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.

3 participants