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

Panic within wrapIfContextDone #802

Closed
joshua-temple opened this issue Dec 16, 2024 · 3 comments
Closed

Panic within wrapIfContextDone #802

joshua-temple opened this issue Dec 16, 2024 · 3 comments
Labels
bug Something isn't working

Comments

@joshua-temple
Copy link

joshua-temple commented Dec 16, 2024

Describe the bug

There appears to be a bug down the client request chain as I am intermittently experiencing a nil panic at connectrpc.com/connect.wrapIfContextDone, to the point I've introduced an intentional recover explicitly for this.

I have even gone so far as to add a check immediately prior to the client call to ensure that context.Context is not nil, and to populate if so. Logs indicate this is never being tripped.

To Reproduce

Unfortunately, I cannot reproduce this.

Additional context

Very straight forward - two services, both using gen'd connect clients/servers.

Service A uses the (connect) client for Service B to make a request, and at times, results in a panic.

These services are bundled, so the gen'd code is all of the same version

  • protoc-gen-go v1.35.1
  • protoc-gen-go-grpc v1.5.1

Our connect imports:

  • connectrpc.com/connect v1.15.0
  • connectrpc.com/grpcreflect v1.2.0

Go v1.22

Stacktrace example:

Note: The recover lives at implementation.go:63

goroutine 105249150 [running]:
runtime/debug.Stack()
	/opt/hostedtoolcache/go/1.22.9/x64/src/runtime/debug/stack.go:24 +0x5e
github.com/redacted/internal/somePackage/implementation.(*someImplementation).SomeMethod.func2.1()
	/home/runner/work/redacted/redacted/internal/somePackage/implementation.go:63 +0x8b
panic({0x22f8d60?, 0x42d5cb0?})
	/opt/hostedtoolcache/go/1.22.9/x64/src/runtime/panic.go:770 +0x132
connectrpc.com/connect.wrapIfContextDone({0x0, 0x0}, {0x2f66c40?, 0xc0051900f0?})
	/home/runner/go/pkg/mod/connectrpc.com/connect@v1.15.0/error.go:321 +0x81
connectrpc.com/connect.(*connectUnaryUnmarshaler).UnmarshalFunc(0xc008e72488, {0x228b560, 0xc0049fed50}, 0x2886f90)
	/home/runner/go/pkg/mod/connectrpc.com/connect@v1.15.0/protocol_connect.go:1118 +0x705
connectrpc.com/connect.(*connectUnaryClientConn).validateResponse(0xc007097380, 0xc009ae2000)
	/home/runner/go/pkg/mod/connectrpc.com/connect@v1.15.0/protocol_connect.go:556 +0x985
connectrpc.com/connect.(*duplexHTTPCall).makeRequest(0xc000562070)
	/home/runner/go/pkg/mod/connectrpc.com/connect@v1.15.0/duplex_http_call.go:320 +0x16c
connectrpc.com/connect.(*duplexHTTPCall).sendUnary(0x30?, {0x2f8de20, 0xc005999980})
	/home/runner/go/pkg/mod/connectrpc.com/connect@v1.15.0/duplex_http_call.go:153 +0x1a9
connectrpc.com/connect.(*duplexHTTPCall).Send(0x7faff6e48688?, {0x2f8de20?, 0xc005999980?})
	/home/runner/go/pkg/mod/connectrpc.com/connect@v1.15.0/duplex_http_call.go:97 +0x27b
connectrpc.com/connect.(*connectUnaryMarshaler).write(0xc007097400, {0xc002f64000, 0xa3, 0x400})
	/home/runner/go/pkg/mod/connectrpc.com/connect@v1.15.0/protocol_connect.go:972 +0xa2
connectrpc.com/connect.(*connectUnaryMarshaler).Marshal(0xc007097400, {0x2572c20, 0xc006aa9f10})
	/home/runner/go/pkg/mod/connectrpc.com/connect@v1.15.0/protocol_connect.go:956 +0x3a5
connectrpc.com/connect.(*connectUnaryRequestMarshaler).Marshal(0x0?, {0x2572c20?, 0xc006aa9f10?})
	/home/runner/go/pkg/mod/connectrpc.com/connect@v1.15.0/protocol_connect.go:1001 +0xf0
connectrpc.com/connect.(*connectUnaryClientConn).Send(0xb5dd9b?, {0x2572c20?, 0xc006aa9f10?})
	/home/runner/go/pkg/mod/connectrpc.com/connect@v1.15.0/protocol_connect.go:474 +0x25
connectrpc.com/connect.(*errorTranslatingClientConn).Send(0xc008ad92f0, {0x2572c20?, 0xc006aa9f10?})
	/home/runner/go/pkg/mod/connectrpc.com/connect@v1.15.0/protocol.go:205 +0x2d
connectrpc.com/connect.NewClient[...].func1({0x2f97ff0, 0xc0088f0180})
	/home/runner/go/pkg/mod/connectrpc.com/connect@v1.15.0/client.go:86 +0x185
connectrpc.com/connect.NewClient[...].func2(0xc0088f0180)
	/home/runner/go/pkg/mod/connectrpc.com/connect@v1.15.0/client.go:112 +0x1ba
connectrpc.com/connect.(*Client[...]).CallUnary(0x412145?, {0x2f8d8c8?, 0xc0059997d0?}, 0xc00846c580?)
	/home/runner/go/pkg/mod/connectrpc.com/connect@v1.15.0/client.go:130 +0x44
github.com/redacted/gen/go/redacted/service/v1/servicev1connect.(*serviceClient).DoSomething(0x2507c80?, {0x2f8d8c8?, 0xc0059997d0?}, 0xd?)
	/home/runner/work/redacted/redacted/gen/go/redacted/service/v1/servicev1connect/service_service.connect.go:104 +0x30
github.com/redacted/pkg/client/service.(*client).DoSomething(0xc001362210, {0x2f8d8c8?, 0xc006517c80?}, {0xc004ebf2e7, 0x5}, {0xc003029aa0, 0x24}, {0xc003556a08, 0x1, 0x1}, ...)
@joshua-temple joshua-temple added the bug Something isn't working label Dec 16, 2024
@joshua-temple
Copy link
Author

I may have been pre-emptive in this, updated to 1.17 connect.

I'll report back if this trips again, will close this if nothing pops through the week.

@jhump
Copy link
Member

jhump commented Dec 16, 2024

I suspect this is fixed. It looks like in v1.15.0, this was possible because of this code, that constructs a connectUnaryUnmarshaler but fails to set the ctx field. So, when that field is nil, it could later produce the panic stack-trace above.

But that was fixed in #711 here, which was released in v1.16.0.

@chrispine
Copy link
Contributor

As we believe this is fixed, I'm going to go ahead and close this. If you still have issues, though, let us know. Thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

3 participants