Private
Public Access
Actually fix WNS push, and stop the notification toggle hanging forever (#56)
The pywebpush version bump alone didn't fix WNS: even the latest release (2.4.0) has no WNS-specific header handling in its own source, confirmed by inspecting the installed package directly. Adds the required X-WNS-Cache-Policy header ourselves via webpush()'s own headers= param, gated to *.notify.windows.com endpoints. Also: subscribeToPush()'s permission request and service-worker-ready wait had no timeout, so a browser that never settles either (seen live on a fresh Windows/Edge install -- greyed out, no prompt, no error) left the toggle stuck forever with no feedback. Both now time out after 20s with an actionable message instead. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
This commit is contained in:
@@ -2,6 +2,7 @@ import asyncio
|
|||||||
import json
|
import json
|
||||||
import logging
|
import logging
|
||||||
import uuid
|
import uuid
|
||||||
|
from urllib.parse import urlparse
|
||||||
|
|
||||||
from pywebpush import WebPushException, webpush
|
from pywebpush import WebPushException, webpush
|
||||||
from sqlalchemy import delete, select
|
from sqlalchemy import delete, select
|
||||||
@@ -53,7 +54,23 @@ async def unsubscribe(db: AsyncSession, user_id: uuid.UUID, endpoint: str) -> No
|
|||||||
await db.commit()
|
await db.commit()
|
||||||
|
|
||||||
|
|
||||||
|
# #56 correction: bumping to pywebpush's latest release (2.4.0) turned out
|
||||||
|
# not to actually fix WNS -- checked the installed package's own source
|
||||||
|
# directly and it has no WNS-specific code anywhere; the upstream
|
||||||
|
# discussion (web-push-libs/pywebpush#162) apparently never shipped.
|
||||||
|
# Worked around here instead, using the `headers` param webpush() already
|
||||||
|
# exposes for exactly this: WNS (Windows/Edge push,
|
||||||
|
# *.notify.windows.com) has required this header since April 2024, or it
|
||||||
|
# 400s with no useful body -- "cache" for a non-zero TTL, "no-cache" for
|
||||||
|
# zero (this app never sets a TTL, so always the latter).
|
||||||
|
def _is_wns_endpoint(endpoint: str) -> bool:
|
||||||
|
return urlparse(endpoint).hostname is not None and urlparse(endpoint).hostname.endswith(
|
||||||
|
"notify.windows.com"
|
||||||
|
)
|
||||||
|
|
||||||
|
|
||||||
def _send_one(subscription: PushSubscription, payload: dict) -> None:
|
def _send_one(subscription: PushSubscription, payload: dict) -> None:
|
||||||
|
extra_headers = {"X-WNS-Cache-Policy": "no-cache"} if _is_wns_endpoint(subscription.endpoint) else None
|
||||||
webpush(
|
webpush(
|
||||||
subscription_info={
|
subscription_info={
|
||||||
"endpoint": subscription.endpoint,
|
"endpoint": subscription.endpoint,
|
||||||
@@ -62,6 +79,7 @@ def _send_one(subscription: PushSubscription, payload: dict) -> None:
|
|||||||
data=json.dumps(payload),
|
data=json.dumps(payload),
|
||||||
vapid_private_key=settings.vapid_private_key,
|
vapid_private_key=settings.vapid_private_key,
|
||||||
vapid_claims={"sub": settings.vapid_subject},
|
vapid_claims={"sub": settings.vapid_subject},
|
||||||
|
headers=extra_headers,
|
||||||
)
|
)
|
||||||
|
|
||||||
|
|
||||||
|
|||||||
@@ -15,13 +15,13 @@ dependencies = [
|
|||||||
"pydantic-settings>=2.6",
|
"pydantic-settings>=2.6",
|
||||||
"argon2-cffi>=23.1",
|
"argon2-cffi>=23.1",
|
||||||
"itsdangerous>=2.2",
|
"itsdangerous>=2.2",
|
||||||
# #56: >=2.0 let the production venv sit on an old 2.0.x that predates
|
# #56: >=2.0 let the production venv sit on an old 2.0.x with no real
|
||||||
# the fix for web-push-libs/pywebpush#162 -- WNS (Windows/Edge push)
|
# downside to bumping the floor -- worth keeping current regardless.
|
||||||
# has required an X-WNS-Cache-Policy header since April 2024, and an
|
# Doesn't by itself fix WNS (Windows/Edge push): despite
|
||||||
# old pywebpush that never sends it gets a bodyless 400 on every send.
|
# web-push-libs/pywebpush#162's discussion, even the latest release
|
||||||
# Floored at the actual latest release (merged/shipped 2026-01 through
|
# (2.4.0) has no WNS-specific header handling in its own source. The
|
||||||
# 2026-08) rather than pinning to whichever exact point release first
|
# actual fix is app/services/push_service.py adding the required
|
||||||
# included the fix, since that wasn't independently confirmed.
|
# X-WNS-Cache-Policy header itself via webpush()'s `headers` param.
|
||||||
"pywebpush>=2.4.0",
|
"pywebpush>=2.4.0",
|
||||||
"redis>=5.0",
|
"redis>=5.0",
|
||||||
"httpx>=0.27",
|
"httpx>=0.27",
|
||||||
|
|||||||
@@ -272,3 +272,61 @@ def test_non_gone_push_failure_logs_response_detail_and_keeps_subscription(ws_cl
|
|||||||
assert len(calls) == 1
|
assert len(calls) == 1
|
||||||
assert "Bad Request" in calls[0]
|
assert "Bad Request" in calls[0]
|
||||||
assert "Ttl value conflicts with X-WNS-Cache-Policy" in calls[0]
|
assert "Ttl value conflicts with X-WNS-Cache-Policy" in calls[0]
|
||||||
|
|
||||||
|
|
||||||
|
def test_wns_endpoint_gets_cache_policy_header(ws_client, monkeypatch):
|
||||||
|
calls = []
|
||||||
|
monkeypatch.setattr("app.services.push_service.webpush", lambda **kw: calls.append(kw))
|
||||||
|
|
||||||
|
alice = _register_ws(ws_client, _unique("alice"))
|
||||||
|
room = ws_client.post("/api/rooms", json={"name": _unique("general")}).json()
|
||||||
|
|
||||||
|
bob = _register_ws(ws_client, _unique("bob"))
|
||||||
|
ws_client.post(f"/api/rooms/{room['id']}/join")
|
||||||
|
ws_client.post(
|
||||||
|
"/api/push/subscribe",
|
||||||
|
json={
|
||||||
|
"endpoint": f"https://wns2-by3p.notify.windows.com/w/{_unique('bob')}",
|
||||||
|
"keys": {"p256dh": "p256dh-bob", "auth": "auth-bob"},
|
||||||
|
},
|
||||||
|
)
|
||||||
|
|
||||||
|
ws_client.post(
|
||||||
|
"/api/auth/login", json={"username_or_email": alice["username"], "password": "password123"}
|
||||||
|
)
|
||||||
|
with ws_client.websocket_connect("/ws/chat") as ws:
|
||||||
|
ws.send_json({"type": "join", "room_id": room["id"]})
|
||||||
|
assert ws.receive_json()["type"] == "joined"
|
||||||
|
ws.send_json({"type": "message", "room_id": room["id"], "content": "hello"})
|
||||||
|
assert ws.receive_json()["type"] == "message"
|
||||||
|
ws.send_json({"type": "join", "room_id": room["id"]})
|
||||||
|
assert ws.receive_json()["type"] == "joined"
|
||||||
|
|
||||||
|
assert len(calls) == 1
|
||||||
|
assert calls[0]["headers"] == {"X-WNS-Cache-Policy": "no-cache"}
|
||||||
|
|
||||||
|
|
||||||
|
def test_non_wns_endpoint_gets_no_extra_headers(ws_client, monkeypatch):
|
||||||
|
calls = []
|
||||||
|
monkeypatch.setattr("app.services.push_service.webpush", lambda **kw: calls.append(kw))
|
||||||
|
|
||||||
|
alice = _register_ws(ws_client, _unique("alice"))
|
||||||
|
room = ws_client.post("/api/rooms", json={"name": _unique("general")}).json()
|
||||||
|
|
||||||
|
bob = _register_ws(ws_client, _unique("bob"))
|
||||||
|
ws_client.post(f"/api/rooms/{room['id']}/join")
|
||||||
|
ws_client.post("/api/push/subscribe", json=_subscription_payload(_unique("bob")))
|
||||||
|
|
||||||
|
ws_client.post(
|
||||||
|
"/api/auth/login", json={"username_or_email": alice["username"], "password": "password123"}
|
||||||
|
)
|
||||||
|
with ws_client.websocket_connect("/ws/chat") as ws:
|
||||||
|
ws.send_json({"type": "join", "room_id": room["id"]})
|
||||||
|
assert ws.receive_json()["type"] == "joined"
|
||||||
|
ws.send_json({"type": "message", "room_id": room["id"], "content": "hello"})
|
||||||
|
assert ws.receive_json()["type"] == "message"
|
||||||
|
ws.send_json({"type": "join", "room_id": room["id"]})
|
||||||
|
assert ws.receive_json()["type"] == "joined"
|
||||||
|
|
||||||
|
assert len(calls) == 1
|
||||||
|
assert calls[0]["headers"] is None
|
||||||
|
|||||||
@@ -1,5 +1,33 @@
|
|||||||
import { getVapidPublicKey, subscribePush, unsubscribePush } from '../api/push'
|
import { getVapidPublicKey, subscribePush, unsubscribePush } from '../api/push'
|
||||||
|
|
||||||
|
// A stuck permission prompt or service-worker-ready wait would otherwise
|
||||||
|
// leave TopBar's "Enable notifications" toggle permanently disabled with
|
||||||
|
// no feedback at all -- confirmed live on a fresh Windows/Edge install
|
||||||
|
// (never been used before): clicking it greyed the button out and never
|
||||||
|
// showed the OS permission prompt, with no error and no way out short of
|
||||||
|
// reloading. Most likely cause is Windows' own per-app notification
|
||||||
|
// permission being off for Edge (Settings > System > Notifications), which
|
||||||
|
// some Chromium versions handle by just never resolving the request
|
||||||
|
// instead of rejecting it -- but whatever the cause, the UI should never
|
||||||
|
// hang forever waiting on a browser API that may simply never settle.
|
||||||
|
class PushTimeoutError extends Error {}
|
||||||
|
|
||||||
|
function withTimeout<T>(promise: Promise<T>, ms: number, message: string): Promise<T> {
|
||||||
|
return new Promise((resolve, reject) => {
|
||||||
|
const timer = setTimeout(() => reject(new PushTimeoutError(message)), ms)
|
||||||
|
promise.then(
|
||||||
|
(value) => {
|
||||||
|
clearTimeout(timer)
|
||||||
|
resolve(value)
|
||||||
|
},
|
||||||
|
(err: unknown) => {
|
||||||
|
clearTimeout(timer)
|
||||||
|
reject(err)
|
||||||
|
},
|
||||||
|
)
|
||||||
|
})
|
||||||
|
}
|
||||||
|
|
||||||
function urlBase64ToUint8Array(base64String: string): Uint8Array<ArrayBuffer> {
|
function urlBase64ToUint8Array(base64String: string): Uint8Array<ArrayBuffer> {
|
||||||
const padding = '='.repeat((4 - (base64String.length % 4)) % 4)
|
const padding = '='.repeat((4 - (base64String.length % 4)) % 4)
|
||||||
const base64 = (base64String + padding).replace(/-/g, '+').replace(/_/g, '/')
|
const base64 = (base64String + padding).replace(/-/g, '+').replace(/_/g, '/')
|
||||||
@@ -27,7 +55,11 @@ export async function subscribeToPush(): Promise<void> {
|
|||||||
throw new Error('Push notifications are not supported in this browser')
|
throw new Error('Push notifications are not supported in this browser')
|
||||||
}
|
}
|
||||||
|
|
||||||
const permission = await Notification.requestPermission()
|
const permission = await withTimeout(
|
||||||
|
Notification.requestPermission(),
|
||||||
|
20_000,
|
||||||
|
"The browser never responded to the notification permission request. Check this browser's notification permission for this site, and your OS-level notification settings for the browser, then try again.",
|
||||||
|
)
|
||||||
if (permission !== 'granted') {
|
if (permission !== 'granted') {
|
||||||
throw new Error('Notification permission was not granted')
|
throw new Error('Notification permission was not granted')
|
||||||
}
|
}
|
||||||
@@ -37,7 +69,11 @@ export async function subscribeToPush(): Promise<void> {
|
|||||||
throw new Error('Push notifications are not configured on the server')
|
throw new Error('Push notifications are not configured on the server')
|
||||||
}
|
}
|
||||||
|
|
||||||
const registration = await navigator.serviceWorker.ready
|
const registration = await withTimeout(
|
||||||
|
navigator.serviceWorker.ready,
|
||||||
|
20_000,
|
||||||
|
'The browser never finished setting up its background service worker. Try reloading the page.',
|
||||||
|
)
|
||||||
const subscription = await registration.pushManager.subscribe({
|
const subscription = await registration.pushManager.subscribe({
|
||||||
userVisibleOnly: true,
|
userVisibleOnly: true,
|
||||||
applicationServerKey: urlBase64ToUint8Array(public_key),
|
applicationServerKey: urlBase64ToUint8Array(public_key),
|
||||||
|
|||||||
Reference in New Issue
Block a user