yarpc / yarpc-go

A message passing platform for Go
MIT License
401 stars 101 forks source link

Fixed panic when error details contain unmarshallable message #2234

Closed biosvs closed 7 months ago

biosvs commented 7 months ago

GetErrorDetails may return both proto.Message and std error objects, while setApplicationErrorMeta expects to see only proto.Message object. As a result it may lead to panic while casting std error to proto.Message.

New unit-test proofs that panic exist without fix. There is no panic with presented fix.

Output of the new test against the code without fix:

> GOTOOLCHAIN=go1.21.0 go test -v -run=TestSetApplicationErrorMeta .
=== RUN   TestSetApplicationErrorMeta
--- FAIL: TestSetApplicationErrorMeta (0.00s)
panic: interface conversion: *errors.errorString is not proto.Message: missing method ProtoMessage [recovered]
        panic: interface conversion: *errors.errorString is not proto.Message: missing method ProtoMessage

goroutine 35 [running]:
testing.tRunner.func1.2({0x1734b00, 0xc0000bcff0})
        /Users/vitalii/go/pkg/mod/golang.org/toolchain@v0.0.1-go1.21.0.darwin-amd64/src/testing/testing.go:1545 +0x238
testing.tRunner.func1()
        /Users/vitalii/go/pkg/mod/golang.org/toolchain@v0.0.1-go1.21.0.darwin-amd64/src/testing/testing.go:1548 +0x397
panic({0x1734b00?, 0xc0000bcff0?})
        /Users/vitalii/go/pkg/mod/golang.org/toolchain@v0.0.1-go1.21.0.darwin-amd64/src/runtime/panic.go:914 +0x21f
go.uber.org/yarpc/encoding/protobuf.setApplicationErrorMeta(0xc0000ba210?, {0x191dc60?, 0xc0000ce230?})
        /Users/vitalii/go/src/github.com/biosvs/yarpc-go/encoding/protobuf/error.go:182 +0xa5
go.uber.org/yarpc/encoding/protobuf.TestSetApplicationErrorMeta(0x0?)
        /Users/vitalii/go/src/github.com/biosvs/yarpc-go/encoding/protobuf/error_test.go:286 +0x237
testing.tRunner(0xc0000a2b60, 0x18620a0)
        /Users/vitalii/go/pkg/mod/golang.org/toolchain@v0.0.1-go1.21.0.darwin-amd64/src/testing/testing.go:1595 +0xff
created by testing.(*T).Run in goroutine 1
        /Users/vitalii/go/pkg/mod/golang.org/toolchain@v0.0.1-go1.21.0.darwin-amd64/src/testing/testing.go:1648 +0x3ad
FAIL    go.uber.org/yarpc/encoding/protobuf     0.358s
FAIL
CLAassistant commented 7 months ago

CLA assistant check
All committers have signed the CLA.