diff --git a/server/internal/api/api.go b/server/internal/api/api.go index 2a0ea4e..b3059d4 100644 --- a/server/internal/api/api.go +++ b/server/internal/api/api.go @@ -34,7 +34,7 @@ func NewHandler(st *store.Store, tm *auth.TokenManager, hub *realtime.Hub, log * log: log, secure: cfg.Environment == "production", media: ms, - notifier: newPushNotifier(st, hub, cfg.VAPIDPublicKey, cfg.VAPIDPrivateKey, cfg.VAPIDSubject), + notifier: newPushNotifier(st, hub, log, cfg.VAPIDPublicKey, cfg.VAPIDPrivateKey, cfg.VAPIDSubject), } mux := http.NewServeMux() diff --git a/server/internal/api/push.go b/server/internal/api/push.go index ca6eb69..761eb82 100644 --- a/server/internal/api/push.go +++ b/server/internal/api/push.go @@ -3,8 +3,11 @@ package api import ( "context" "encoding/json" + "io" + "log/slog" "net/http" "strings" + "time" "github.com/SherClockHolmes/webpush-go" @@ -63,16 +66,17 @@ func (h *apiHandler) unsubscribePush(w http.ResponseWriter, r *http.Request) { type pushNotifier struct { store *store.Store hub *realtime.Hub + log *slog.Logger public string private string subject string } -func newPushNotifier(st *store.Store, hub *realtime.Hub, public, private, subject string) *pushNotifier { +func newPushNotifier(st *store.Store, hub *realtime.Hub, log *slog.Logger, public, private, subject string) *pushNotifier { if public == "" || private == "" { return nil } - return &pushNotifier{store: st, hub: hub, public: public, private: private, subject: subject} + return &pushNotifier{store: st, hub: hub, log: log, public: public, private: private, subject: subject} } // notifyMembers pushes a message notification to offline members. @@ -106,12 +110,17 @@ func (n *pushNotifier) notifyMembers(ctx context.Context, conversationID, sender continue } for _, sub := range subs { - go n.send(ctx, m.User.ID, sub, payload) + go n.send(m.User.ID, sub, payload) } } } -func (n *pushNotifier) send(ctx context.Context, userID int64, sub store.PushSubscription, payload []byte) { +// send delivers a single notification using a detached context, because the +// originating HTTP request context is canceled as soon as its handler returns. +func (n *pushNotifier) send(userID int64, sub store.PushSubscription, payload []byte) { + ctx, cancel := context.WithTimeout(context.Background(), 15*time.Second) + defer cancel() + wp := &webpush.Subscription{Endpoint: sub.Endpoint, Keys: webpush.Keys{P256dh: sub.P256dh, Auth: sub.Auth}} resp, err := webpush.SendNotificationWithContext(ctx, payload, wp, &webpush.Options{ VAPIDPublicKey: n.public, @@ -120,10 +129,15 @@ func (n *pushNotifier) send(ctx context.Context, userID int64, sub store.PushSub TTL: 24 * 60 * 60, }) if err != nil { + n.log.Warn("push send failed", "user_id", userID, "error", err) return } defer resp.Body.Close() - if resp.StatusCode == http.StatusNotFound || resp.StatusCode == http.StatusGone { + switch { + case resp.StatusCode == http.StatusNotFound || resp.StatusCode == http.StatusGone: _ = n.store.DeletePushSubscription(context.Background(), userID, sub.Endpoint) + case resp.StatusCode >= 300: + body, _ := io.ReadAll(io.LimitReader(resp.Body, 512)) + n.log.Warn("push rejected", "user_id", userID, "status", resp.StatusCode, "body", string(body)) } }