From cf21668782fe559026d9a96807dbb3a7eacde2e9 Mon Sep 17 00:00:00 2001 From: anonpenguin23 Date: Fri, 12 Jun 2026 17:49:44 +0300 Subject: [PATCH] fix(push): cap VoIP apns-expiration to the ring window; record success status (#132) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit VoIP call-invite pushes set no apns-expiration, so apns2 omits the header and APNs store-and-forwards the push — delivering it minutes late and firing a phantom "missed call" ring long after the call ended (and burning PushKit goodwill, inviting throttling). Cap the VoIP apns-expiration to the ring window (30s) so APNs delivers promptly or DISCARDS, never a stale invite. Alert pushes keep the default store-and-forward so a message notification still lands after the device reconnects. Also surface HTTP 200 on a successful dispatch instead of leaving HTTPStatus at 0 — a successful push was logging "http=0", which reads like an opaque failure and masked real false-success classes. Tests: VoIP push carries an expiration within the ring-window cap; alert push carries none. push package green. --- core/pkg/push/dispatcher.go | 7 ++++ core/pkg/push/providers/apns/apns.go | 18 ++++++++++ core/pkg/push/providers/apns/voip_test.go | 41 +++++++++++++++++++++++ 3 files changed, 66 insertions(+) diff --git a/core/pkg/push/dispatcher.go b/core/pkg/push/dispatcher.go index 9d786c9..f9b15e8 100644 --- a/core/pkg/push/dispatcher.go +++ b/core/pkg/push/dispatcher.go @@ -4,6 +4,7 @@ import ( "context" "errors" "fmt" + "net/http" "sync" "go.uber.org/zap" @@ -185,6 +186,12 @@ func (d *PushDispatcher) SendToUserDetailed( out.Ok = false } else { r.Success = true + // Record the success status explicitly. A provider Send returns nil + // only on a 2xx delivery, so surface 200 instead of leaving + // HTTPStatus at its zero value — otherwise a successful push logs + // "http=0", which reads like an opaque failure and masks real + // false-success classes (bugboard #132). + r.HTTPStatus = http.StatusOK out.DevicesSucceeded++ } out.Results = append(out.Results, r) diff --git a/core/pkg/push/providers/apns/apns.go b/core/pkg/push/providers/apns/apns.go index e73049b..d5868e0 100644 --- a/core/pkg/push/providers/apns/apns.go +++ b/core/pkg/push/providers/apns/apns.go @@ -20,6 +20,15 @@ import ( // provider's 5s because APNs is HTTP/2 + connection-reused. const defaultSendTimeout = 10 * time.Second +// voipPushExpiry caps the apns-expiration on VoIP (call-invite) pushes to the +// ring window. A call signal that can't be delivered within this window is +// worse than undelivered: without an expiration APNs store-and-forwards it and +// lands it MINUTES later, firing a phantom "missed call" ring on the device and +// burning PushKit goodwill (bugboard #132). With it, APNs delivers promptly or +// DISCARDS — never a stale invite. Alert pushes keep the default +// store-and-forward behavior. +const voipPushExpiry = 30 * time.Second + // Provider is the APNs push.PushProvider implementation, scoped to one // (Team ID, Key ID, p8 key, Bundle ID, Environment, Kind) tuple. // Construct one per (namespace, kind) via the gateway dependency @@ -169,6 +178,15 @@ func (p *Provider) Send(ctx context.Context, msg push.PushMessage) error { n.Priority = apns2.PriorityLow } + // Cap VoIP expiration to the ring window so APNs never store-and-forwards a + // stale call-invite into a phantom missed-call ring (bugboard #132). Without + // this, apns2 omits apns-expiration and APNs stores+retries for its default + // (minutes to days). Alert pushes intentionally keep the default so a + // message notification still lands after the device reconnects. + if p.kind == KindVoIP { + n.Expiration = time.Now().Add(voipPushExpiry) + } + // PushWithContext propagates cancellation through to the HTTP/2 // stream — abandoning ctx terminates the in-flight request, no // goroutine leak. diff --git a/core/pkg/push/providers/apns/voip_test.go b/core/pkg/push/providers/apns/voip_test.go index aa6ed24..1a2db9e 100644 --- a/core/pkg/push/providers/apns/voip_test.go +++ b/core/pkg/push/providers/apns/voip_test.go @@ -4,6 +4,7 @@ import ( "context" "net/http" "testing" + "time" "github.com/DeBrosOfficial/network/pkg/push" "github.com/sideshow/apns2" @@ -185,3 +186,43 @@ func TestAlert_Send_EmptyContentStillRejected(t *testing.T) { t.Fatal("alert path should still reject empty-content (bugboard #348); got nil") } } + +// Bugboard #132: VoIP call-invites MUST carry a short apns-expiration so APNs +// never store-and-forwards a stale invite into a phantom missed-call ring +// minutes later. Without it apns2 omits the header → store-and-forward. +func TestVoIP_Send_ExpirationCappedToRingWindow(t *testing.T) { + fake := &fakePushClient{resp: &apns2.Response{StatusCode: http.StatusOK, ApnsID: "voip-exp"}} + p := newTestProviderKind(t, "com.example.app", KindVoIP, fake) + before := time.Now() + if err := p.Send(context.Background(), push.PushMessage{ + DeviceToken: "VOIP-TOKEN", + Data: map[string]interface{}{"call_id": "x"}, + }); err != nil { + t.Fatalf("Send: %v", err) + } + exp := fake.lastSent.Expiration + if exp.IsZero() { + t.Fatal("VoIP push has NO apns-expiration — APNs store-and-forwards → late phantom ring (#132)") + } + if !exp.After(before) { + t.Errorf("expiration %v not in the future (before=%v)", exp, before) + } + if exp.After(before.Add(voipPushExpiry + 2*time.Second)) { + t.Errorf("expiration %v exceeds the ring-window cap (%s) — would allow a late ring", exp, voipPushExpiry) + } +} + +// Alert (message) pushes intentionally keep store-and-forward (no expiration) so +// a notification still lands after reconnect — only the VoIP path is capped. +func TestAlert_Send_NoExpiration_keepsStoreAndForward(t *testing.T) { + fake := &fakePushClient{resp: &apns2.Response{StatusCode: http.StatusOK, ApnsID: "alert-1"}} + p := newTestProviderKind(t, "com.example.app", KindAlert, fake) + if err := p.Send(context.Background(), push.PushMessage{ + DeviceToken: "ALERT-TOKEN", Title: "hi", Body: "msg", + }); err != nil { + t.Fatalf("Send: %v", err) + } + if !fake.lastSent.Expiration.IsZero() { + t.Errorf("alert push set expiration %v; want none (store-and-forward)", fake.lastSent.Expiration) + } +}