Skip to content

Fix query_range error handling panic #5667

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.

Sign up for GitHub

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

Jump to bottom
Merged
mdisibio merged 2 commits into from
Sep 19, 2025
Merged

Fix query_range error handling panic #5667

mdisibio merged 2 commits into from
Sep 19, 2025

Conversation

@mdisibio
Copy link
Contributor

@mdisibio mdisibio commented Sep 19, 2025 •
edited
Loading

What this PR does:
It was possible to make the query_range endpoint panic under certain conditions. This was due to a defer() which always wrote the response regardless of errors, and when the response was nil, would panic inside proto marshaling.

This endpoint was different than the others because it has span instrumentation and the defer was related to it. Fixed by making this endpoint work like the others, using the common handleError() method and limiting defer to only the necessary for the span instrumentation.

panic: runtime error: invalid memory address or nil pointer dereference [recovered, repanicked]
[signal SIGSEGV: segmentation violation code=0x1 addr=0x30 pc=0x10dd5e6]

goroutine 4767 [running]:
github.com/grafana/tempo/pkg/tempopb.(*QueryRangeResponse).MarshalToSizedBuffer(0x0, {0x6755860, 0x0, 0x0})
	/home/runner/work/tempo/tempo/pkg/tempopb/tempo.pb.go:7600 +0x26
github.com/grafana/tempo/pkg/tempopb.(*QueryRangeResponse).Marshal(0x0)
	/home/runner/work/tempo/tempo/pkg/tempopb/tempo.pb.go:7583 +0x4d
google.golang.org/protobuf/internal/impl.legacyMarshal({{}, {0x436c9c0, 0xc058f28c70}, {0x0, 0x0, 0x0}, 0x0})
	/home/runner/work/tempo/tempo/vendor/google.golang.org/protobuf/internal/impl/legacy_message.go:408 +0xf2
google.golang.org/protobuf/proto.MarshalOptions.marshal({{}, 0x28?, 0x1c?, 0x71?}, {0x0, 0x0, 0x0}, {0x436c9c0, 0xc058f28c70})
	/home/runner/work/tempo/tempo/vendor/google.golang.org/protobuf/proto/encode.go:195 +0x35d
google.golang.org/protobuf/proto.MarshalOptions.MarshalAppend({{}, 0xa0?, 0xba?, 0xaa?}, {0x0, 0x0, 0x0}, {0x42e5d20?, 0xc058f28c70?})
	/home/runner/work/tempo/tempo/vendor/google.golang.org/protobuf/proto/encode.go:159 +0x73
github.com/golang/protobuf/proto.marshalAppend({0x0, 0x0, 0x0}, {0x43170b0?, 0x0?}, 0x0)
	/home/runner/work/tempo/tempo/vendor/github.com/golang/protobuf/proto/wire.go:40 +0x92
github.com/golang/protobuf/proto.Marshal(...)
	/home/runner/work/tempo/tempo/vendor/github.com/golang/protobuf/proto/wire.go:23
github.com/grafana/tempo/modules/querier.writeFormattedContentForRequest({0x4319f10, 0xc056cdb200}, 0x435c0b0?, {0x43170b0, 0x0}, {0x435c0b0, 0xc0560c7c20})```

Which issue(s) this PR fixes:
Fixes #

Checklist

  • Tests updated
  • Documentation added
  • CHANGELOG.md updated - the order of entries should be [CHANGE], [FEATURE], [ENHANCEMENT], [BUGFIX]

@mdisibio
Fix query-range error handling panic, by using common handleError met…
f72e4cd 
…hod and deferring to always write a response when it should not be
@mdisibio
changelog
4e80da2
@mdisibio mdisibio marked this pull request as ready for review September 19, 2025 15:32
@mdisibio mdisibio merged commit eb21336 into grafana:main Sep 19, 2025
22 checks passed
@electron0zero electron0zero mentioned this pull request Sep 26, 2025
Closed
@electron0zero electron0zero mentioned this pull request Oct 24, 2025
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@javiermolinar javiermolinar javiermolinar approved these changes

@joe-elliott joe-elliott Awaiting requested review from joe-elliott joe-elliott is a code owner

@mapno mapno Awaiting requested review from mapno mapno is a code owner

@yvrhdn yvrhdn Awaiting requested review from yvrhdn yvrhdn is a code owner

@zalegrala zalegrala Awaiting requested review from zalegrala zalegrala is a code owner

@electron0zero electron0zero Awaiting requested review from electron0zero electron0zero is a code owner

@ie-pham ie-pham Awaiting requested review from ie-pham ie-pham is a code owner

@stoewer stoewer Awaiting requested review from stoewer stoewer is a code owner

@carles-grafana carles-grafana Awaiting requested review from carles-grafana carles-grafana is a code owner

@ruslan-mikhailov ruslan-mikhailov Awaiting requested review from ruslan-mikhailov ruslan-mikhailov is a code owner

@mattdurham mattdurham Awaiting requested review from mattdurham mattdurham is a code owner

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@mdisibio @javiermolinar