Open RaghavSood opened 2 weeks ago
I tried downgrading Sentry to 0.27.0 and upgrading it to 0.28.1 - both have the same issue. Will investigate further.
It has likely been a latent issue for some time now that just surfaced accidentally with flyctl here.
I was able to get it to work as expected with a simple nil check before merge:
diff --git a/transport.go b/transport.go
index 20f6994..4dc50d6 100644
--- a/transport.go
+++ b/transport.go
@@ -690,6 +690,9 @@ func (t *HTTPSyncTransport) SendEventWithContext(ctx context.Context, event *Eve
}
t.mu.Lock()
+ if t.limits == nil {
+ t.limits = make(ratelimit.Map)
+ }
t.limits.Merge(ratelimit.FromResponse(response))
t.mu.Unlock()
For now, I've proposed an upstream fix to set the timeout on top of the default client provided by the go-sdk: https://github.com/superfly/flyctl/pull/3643
This kind of transport initialization only seems to be used by a few consumers, who may not be hitting other conditions required to get the panic (maybe they never end up rate limited?):
https://github.com/search?q=%26sentry.HTTPSyncTransport&type=code https://github.com/search?q=%26sentry.HTTPTransport&type=code
Which would help explain why it went unreported so far
Summary
We're facing an issue packaging https://github.com/superfly/flyctl for nixpkgs due to the way they initialise the sentry SDK.
Their init function defines a non-default
HTTPSyncTransport
. However, they only provide the Timeout parameter, which leaves the rest as nil, including the non-exportedlimits
.This eventually results in a panic when the limits map is merged with the response headers:
This can be reproduced by running:
against flyctl v0.2.71 on at least darwin m1/x86 (I can't speak to how they use sentry on other platforms)
This can also be replicated with the testing suit by applying this:
Since
map.Merge
is a value receiver, not a pointer receiver, it is not possible to handle this and init a new map within the Merge itself.Steps To Reproduce
Apply this patch to the test files:
Then run:
Expected Behavior
Sentry should either not allow a nil limits to occur, or set a sane default within the transport/client call chain
SDK
sentry-go
version: v0.28.0