Skip to content

Commit 260ccbd

Browse files
Revert "feat: expose invalid argument error to clients in bidirectional streaming (#4795) (#4819)" (#5303)
This reverts commit 830ba27. Unfortunately this suffered from a bug that could cause a panic. Revert until we can reintroduce it without the bug.
1 parent c18bc4e commit 260ccbd

File tree

6 files changed

+20
-168
lines changed

6 files changed

+20
-168
lines changed

examples/internal/integration/BUILD.bazel

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -25,7 +25,6 @@ go_test(
2525
"@org_golang_google_protobuf//encoding/protojson",
2626
"@org_golang_google_protobuf//proto",
2727
"@org_golang_google_protobuf//testing/protocmp",
28-
"@org_golang_google_protobuf//types/known/durationpb",
2928
"@org_golang_google_protobuf//types/known/emptypb",
3029
"@org_golang_google_protobuf//types/known/fieldmaskpb",
3130
"@org_golang_google_protobuf//types/known/structpb",

examples/internal/integration/integration_test.go

Lines changed: 0 additions & 94 deletions
Original file line numberDiff line numberDiff line change
@@ -28,7 +28,6 @@ import (
2828
"google.golang.org/protobuf/encoding/protojson"
2929
"google.golang.org/protobuf/proto"
3030
"google.golang.org/protobuf/testing/protocmp"
31-
"google.golang.org/protobuf/types/known/durationpb"
3231
"google.golang.org/protobuf/types/known/emptypb"
3332
fieldmaskpb "google.golang.org/protobuf/types/known/fieldmaskpb"
3433
"google.golang.org/protobuf/types/known/structpb"
@@ -523,7 +522,6 @@ func TestABE(t *testing.T) {
523522
testABEDownload(t, 8088)
524523
testABEBulkEcho(t, 8088)
525524
testABEBulkEchoZeroLength(t, 8088)
526-
testABEBulkEchoDurationError(t, 8088)
527525
testAdditionalBindings(t, 8088)
528526
testABERepeated(t, 8088)
529527
testABEExists(t, 8088)
@@ -1451,98 +1449,6 @@ func testABEBulkEchoZeroLength(t *testing.T, port int) {
14511449
}
14521450
}
14531451

1454-
func testABEBulkEchoDurationError(t *testing.T, port int) {
1455-
reqr, reqw := io.Pipe()
1456-
var wg sync.WaitGroup
1457-
var want []*durationpb.Duration
1458-
wg.Add(1)
1459-
go func() {
1460-
defer wg.Done()
1461-
defer reqw.Close()
1462-
for i := 0; i < 10; i++ {
1463-
s := fmt.Sprintf("%d.123s", i)
1464-
if i == 5 {
1465-
s = "invalidDurationFormat"
1466-
}
1467-
buf, err := marshaler.Marshal(s)
1468-
if err != nil {
1469-
t.Errorf("marshaler.Marshal(%v) failed with %v; want success", s, err)
1470-
return
1471-
}
1472-
if _, err = reqw.Write(buf); err != nil {
1473-
t.Errorf("reqw.Write(%q) failed with %v; want success", string(buf), err)
1474-
return
1475-
}
1476-
want = append(want, &durationpb.Duration{Seconds: int64(i), Nanos: int32(0.123 * 1e9)})
1477-
}
1478-
}()
1479-
apiURL := fmt.Sprintf("http://localhost:%d/v1/example/a_bit_of_everything/echo_duration", port)
1480-
req, err := http.NewRequest("POST", apiURL, reqr)
1481-
if err != nil {
1482-
t.Errorf("http.NewRequest(%q, %q, reqr) failed with %v; want success", "POST", apiURL, err)
1483-
return
1484-
}
1485-
req.Header.Set("Content-Type", "application/json")
1486-
req.Header.Set("Transfer-Encoding", "chunked")
1487-
resp, err := http.DefaultClient.Do(req)
1488-
if err != nil {
1489-
t.Errorf("http.Post(%q, %q, req) failed with %v; want success", apiURL, "application/json", err)
1490-
return
1491-
}
1492-
defer resp.Body.Close()
1493-
if got, want := resp.StatusCode, http.StatusOK; got != want {
1494-
t.Errorf("resp.StatusCode = %d; want %d", got, want)
1495-
}
1496-
1497-
var got []*durationpb.Duration
1498-
var invalidArgumentCount int
1499-
wg.Add(1)
1500-
go func() {
1501-
defer wg.Done()
1502-
1503-
dec := marshaler.NewDecoder(resp.Body)
1504-
for i := 0; ; i++ {
1505-
var item struct {
1506-
Result json.RawMessage `json:"result"`
1507-
Error map[string]interface{} `json:"error"`
1508-
}
1509-
err := dec.Decode(&item)
1510-
if err == io.EOF {
1511-
break
1512-
}
1513-
if err != nil {
1514-
t.Errorf("dec.Decode(&item) failed with %v; want success; i = %d", err, i)
1515-
}
1516-
if len(item.Error) != 0 {
1517-
code, ok := item.Error["code"].(float64)
1518-
if !ok {
1519-
t.Errorf("item.Error[code] not found or not a number: %#v; i = %d", item.Error, i)
1520-
} else if int32(code) == 3 {
1521-
invalidArgumentCount++
1522-
} else {
1523-
t.Errorf("item.Error[code] = %v; want 3; i = %d", code, i)
1524-
}
1525-
continue
1526-
}
1527-
1528-
msg := new(durationpb.Duration)
1529-
if err := marshaler.Unmarshal(item.Result, msg); err != nil {
1530-
t.Errorf("marshaler.Unmarshal(%q, msg) failed with %v; want success", item.Result, err)
1531-
}
1532-
got = append(got, msg)
1533-
}
1534-
1535-
if invalidArgumentCount != 1 {
1536-
t.Errorf("got %d errors with code 3; want exactly 1", invalidArgumentCount)
1537-
}
1538-
}()
1539-
1540-
wg.Wait()
1541-
if diff := cmp.Diff(got, want[:5], protocmp.Transform()); diff != "" {
1542-
t.Error(diff)
1543-
}
1544-
}
1545-
15461452
func testAdditionalBindings(t *testing.T, port int) {
15471453
for i, f := range []func() *http.Response{
15481454
func() *http.Response {

examples/internal/proto/examplepb/flow_combination.pb.gw.go

Lines changed: 5 additions & 17 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

examples/internal/proto/examplepb/stream.pb.gw.go

Lines changed: 10 additions & 34 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

protoc-gen-grpc-gateway/internal/gengateway/template.go

Lines changed: 4 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -272,7 +272,7 @@ var (
272272

273273
_ = template.Must(handlerTemplate.New("request-func-signature").Parse(strings.ReplaceAll(`
274274
{{ if and .Method.GetClientStreaming .Method.GetServerStreaming }}
275-
func request_{{ .Method.Service.GetName }}_{{ .Method.GetName }}_{{ .Index }}(ctx context.Context, marshaler runtime.Marshaler, client {{ .Method.Service.InstanceName }}Client, req *http.Request, pathParams map[string]string) ({{ .Method.Service.InstanceName }}_{{ .Method.GetName }}Client, runtime.ServerMetadata, chan error, error)
275+
func request_{{ .Method.Service.GetName }}_{{ .Method.GetName }}_{{ .Index }}(ctx context.Context, marshaler runtime.Marshaler, client {{ .Method.Service.InstanceName }}Client, req *http.Request, pathParams map[string]string) ({{ .Method.Service.InstanceName }}_{{ .Method.GetName }}Client, runtime.ServerMetadata, error)
276276
{{ else if .Method.GetServerStreaming }}
277277
func request_{{ .Method.Service.GetName }}_{{ .Method.GetName }}_{{ .Index }}(ctx context.Context, marshaler runtime.Marshaler, client {{ .Method.Service.InstanceName }}Client, req *http.Request, pathParams map[string]string) ({{ .Method.Service.InstanceName }}_{{ .Method.GetName }}Client, runtime.ServerMetadata, error)
278278
{{ else }}
@@ -457,12 +457,10 @@ var filter_{{ .Method.Service.GetName }}_{{ .Method.GetName }}_{{ .Index }} = {{
457457
_ = template.Must(handlerTemplate.New("bidi-streaming-request-func").Parse(`
458458
{{ template "request-func-signature" . }} {
459459
var metadata runtime.ServerMetadata
460-
errChan := make(chan error, 1)
461460
stream, err := client.{{ .Method.GetName }}(ctx)
462461
if err != nil {
463462
grpclog.Errorf("Failed to start streaming: %v", err)
464-
close(errChan)
465-
return nil, metadata, errChan, err
463+
return nil, metadata, err
466464
}
467465
dec := marshaler.NewDecoder(req.Body)
468466
handleSend := func() error {
@@ -482,10 +480,8 @@ var filter_{{ .Method.Service.GetName }}_{{ .Method.GetName }}_{{ .Index }} = {{
482480
return nil
483481
}
484482
go func() {
485-
defer close(errChan)
486483
for {
487484
if err := handleSend(); err != nil {
488-
errChan <- err
489485
break
490486
}
491487
}
@@ -496,10 +492,10 @@ var filter_{{ .Method.Service.GetName }}_{{ .Method.GetName }}_{{ .Index }} = {{
496492
header, err := stream.Header()
497493
if err != nil {
498494
grpclog.Errorf("Failed to get header from client: %v", err)
499-
return nil, metadata, errChan, err
495+
return nil, metadata, err
500496
}
501497
metadata.HeaderMD = header
502-
return stream, metadata, errChan, nil
498+
return stream, metadata, nil
503499
}
504500
`))
505501

@@ -741,25 +737,12 @@ func Register{{ $svc.GetName }}{{ $.RegisterFuncSuffix }}Client(ctx context.Cont
741737
runtime.HTTPError(ctx, mux, outboundMarshaler, w, req, err)
742738
return
743739
}
744-
{{if and $m.GetClientStreaming $m.GetServerStreaming }}
745-
resp, md, reqErrChan, err := request_{{ $svc.GetName }}_{{ $m.GetName }}_{{ $b.Index }}(annotatedContext, inboundMarshaler, client, req, pathParams)
746-
{{- else -}}
747740
resp, md, err := request_{{ $svc.GetName }}_{{ $m.GetName }}_{{ $b.Index }}(annotatedContext, inboundMarshaler, client, req, pathParams)
748-
{{- end }}
749741
annotatedContext = runtime.NewServerMetadataContext(annotatedContext, md)
750742
if err != nil {
751743
runtime.HTTPError(annotatedContext, mux, outboundMarshaler, w, req, err)
752744
return
753745
}
754-
{{- if and $m.GetClientStreaming $m.GetServerStreaming }}
755-
go func() {
756-
for err := range reqErrChan {
757-
if err != nil && !errors.Is(err, io.EOF) {
758-
runtime.HTTPStreamError(annotatedContext, mux, outboundMarshaler, w, req, err)
759-
}
760-
}
761-
}()
762-
{{- end }}
763746
{{- if $m.GetServerStreaming }}
764747
{{- if $b.ResponseBody }}
765748
forward_{{ $svc.GetName }}_{{ $m.GetName }}_{{ $b.Index }}(annotatedContext, mux, outboundMarshaler, w, req, func() (proto.Message, error) {

protoc-gen-grpc-gateway/internal/gengateway/template_test.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -322,7 +322,7 @@ func TestApplyTemplateRequestWithClientStreaming(t *testing.T) {
322322
},
323323
{
324324
serverStreaming: true,
325-
sigWant: `func request_ExampleService_Echo_0(ctx context.Context, marshaler runtime.Marshaler, client ExampleServiceClient, req *http.Request, pathParams map[string]string) (ExampleService_EchoClient, runtime.ServerMetadata, chan error, error) {`,
325+
sigWant: `func request_ExampleService_Echo_0(ctx context.Context, marshaler runtime.Marshaler, client ExampleServiceClient, req *http.Request, pathParams map[string]string) (ExampleService_EchoClient, runtime.ServerMetadata, error) {`,
326326
},
327327
} {
328328
meth.ServerStreaming = proto.Bool(spec.serverStreaming)

0 commit comments

Comments
 (0)