mirror of
https://github.com/karatektus/pl_mail.git
synced 2026-10-25 10:04:18 +00:00
Say which step failed when notifications can't be turned on (#278)
Every failure in the enable path (registering the service worker, the browser's push subscription, the server's answer) ended in the same "Something went wrong" and the exception was dropped, so a report from a Pixel 8 could not say which. The pane now names the step and the DOMException name or HTTP status, and logs the exception to the console. Closes #277 Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
This commit is contained in:
1 parent
ed4d02153d
commit
408ea47e95
8 files changed
+162
-4
No files matched your search
@@ -64,10 +64,18 @@ export default class extends Controller {
|
|||||||
return;
|
return;
|
||||||
}
|
}
|
||||||
|
|
||||||
|
// Which step failed is the whole diagnosis. Registering the worker,
|
||||||
|
// subscribing and the server's answer break for different reasons
|
||||||
|
// (a proxy, the push service, CSRF or configuration), and one shared
|
||||||
|
// "something went wrong" left a report saying nothing about which.
|
||||||
|
let step = "worker";
|
||||||
|
|
||||||
try {
|
try {
|
||||||
const registration = await navigator.serviceWorker.register("/sw.js", { scope: "/" });
|
const registration = await navigator.serviceWorker.register("/sw.js", { scope: "/" });
|
||||||
await navigator.serviceWorker.ready;
|
await navigator.serviceWorker.ready;
|
||||||
|
|
||||||
|
step = "subscribe";
|
||||||
|
|
||||||
// An existing subscription may be bound to a rotated VAPID key, in
|
// An existing subscription may be bound to a rotated VAPID key, in
|
||||||
// which case subscribe() throws; dropping it first is cheaper than
|
// which case subscribe() throws; dropping it first is cheaper than
|
||||||
// parsing the error.
|
// parsing the error.
|
||||||
@@ -84,6 +92,8 @@ export default class extends Controller {
|
|||||||
|
|
||||||
const json = subscription.toJSON();
|
const json = subscription.toJSON();
|
||||||
|
|
||||||
|
step = "server";
|
||||||
|
|
||||||
const response = await fetch(this.subscribeUrlValue, {
|
const response = await fetch(this.subscribeUrlValue, {
|
||||||
method: "POST",
|
method: "POST",
|
||||||
credentials: "same-origin",
|
credentials: "same-origin",
|
||||||
@@ -99,7 +109,7 @@ export default class extends Controller {
|
|||||||
});
|
});
|
||||||
|
|
||||||
if (false === response.ok) {
|
if (false === response.ok) {
|
||||||
this._setState("error");
|
this._fail(step, `HTTP ${response.status}`);
|
||||||
|
|
||||||
return;
|
return;
|
||||||
}
|
}
|
||||||
@@ -108,7 +118,7 @@ export default class extends Controller {
|
|||||||
this._setState("pending");
|
this._setState("pending");
|
||||||
this._pollStatus(6);
|
this._pollStatus(6);
|
||||||
} catch (e) {
|
} catch (e) {
|
||||||
this._setState("error");
|
this._fail(step, e);
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
@@ -205,13 +215,32 @@ export default class extends Controller {
|
|||||||
}, 1000);
|
}, 1000);
|
||||||
}
|
}
|
||||||
|
|
||||||
_setState(state) {
|
/**
|
||||||
|
* The error state, worded for the step that failed and naming the cause.
|
||||||
|
*
|
||||||
|
* The cause is the DOMException's name (AbortError, InvalidAccessError,
|
||||||
|
* SecurityError…) or the HTTP status: short, the same in every language,
|
||||||
|
* and exactly what a bug report needs. The full exception goes to the
|
||||||
|
* console for anyone who can open it.
|
||||||
|
*/
|
||||||
|
_fail(step, cause) {
|
||||||
|
const detail = cause instanceof Error ? cause.name : String(cause);
|
||||||
|
|
||||||
|
if (cause instanceof Error) {
|
||||||
|
console.error(`plMail: enabling notifications failed at the ${step} step`, cause);
|
||||||
|
}
|
||||||
|
|
||||||
|
this._setState("error", this._label(`error_${step}`).replace("%error%", detail));
|
||||||
|
}
|
||||||
|
|
||||||
|
/** `text` overrides the state's own label, for an error that names its cause. */
|
||||||
|
_setState(state, text = null) {
|
||||||
if (true === this.hasStateTarget) {
|
if (true === this.hasStateTarget) {
|
||||||
this.stateTarget.dataset.pushState = state;
|
this.stateTarget.dataset.pushState = state;
|
||||||
// The data attribute alone is invisible — the label has to be
|
// The data attribute alone is invisible — the label has to be
|
||||||
// written, or the pane sits on its server-rendered "Checking…"
|
// written, or the pane sits on its server-rendered "Checking…"
|
||||||
// forever no matter what the real state is.
|
// forever no matter what the real state is.
|
||||||
this.stateTarget.textContent = this._label(state);
|
this.stateTarget.textContent = text ?? this._label(state);
|
||||||
}
|
}
|
||||||
|
|
||||||
if (true === this.hasToggleTarget) {
|
if (true === this.hasToggleTarget) {
|
||||||
|
|||||||
@@ -0,0 +1,7 @@
|
|||||||
|
**When notifications can't be turned on, Settings now says why.** Every way that turning on
|
||||||
|
notifications can fail used to end in the same "Something went wrong. Try again." Now the pane says
|
||||||
|
which step failed: the browser's background worker, the browser's push service, or the server. It
|
||||||
|
also gives the browser's error name or the server's HTTP status, and writes the full error to the
|
||||||
|
browser console. For Brave, where push needs "Use Google services for push messaging" switched on,
|
||||||
|
the push-service message says so. No schema change.
|
||||||
|
[#277](https://github.com/karatektus/pl_mail/issues/277).
|
||||||
@@ -18,6 +18,9 @@
|
|||||||
working: 'settings.notifications.state_working'|trans,
|
working: 'settings.notifications.state_working'|trans,
|
||||||
denied: 'settings.notifications.state_denied'|trans,
|
denied: 'settings.notifications.state_denied'|trans,
|
||||||
error: 'settings.notifications.state_error'|trans,
|
error: 'settings.notifications.state_error'|trans,
|
||||||
|
error_worker: 'settings.notifications.error_worker'|trans,
|
||||||
|
error_subscribe:'settings.notifications.error_subscribe'|trans,
|
||||||
|
error_server: 'settings.notifications.error_server'|trans,
|
||||||
unsupported: 'settings.notifications.state_unsupported'|trans,
|
unsupported: 'settings.notifications.state_unsupported'|trans,
|
||||||
unconfigured: 'settings.notifications.state_unconfigured'|trans,
|
unconfigured: 'settings.notifications.state_unconfigured'|trans,
|
||||||
action_enable: 'settings.notifications.toggle'|trans,
|
action_enable: 'settings.notifications.toggle'|trans,
|
||||||
|
|||||||
@@ -0,0 +1,107 @@
|
|||||||
|
import { test, expect, type Page } from "./support/test";
|
||||||
|
|
||||||
|
/**
|
||||||
|
* A failed "Enable notifications" says which step failed and why (#277).
|
||||||
|
*
|
||||||
|
* The report it answers: a Pixel 8, Chrome and Brave, permission allowed, and
|
||||||
|
* then at once "Something went wrong. Try again." Every failure in the enable
|
||||||
|
* path landed in that one sentence and the exception was dropped, so neither
|
||||||
|
* the reporter nor anyone reading the report could tell a proxy mangling
|
||||||
|
* /sw.js from a push service refusing the device from the server answering
|
||||||
|
* 403.
|
||||||
|
*
|
||||||
|
* The browser's push machinery is stubbed rather than driven. A real
|
||||||
|
* subscription needs a push service on the other end and VAPID keys the test
|
||||||
|
* stack does not have, and neither is what is being tested: the question is
|
||||||
|
* only what the pane says when one of the three steps throws or is refused.
|
||||||
|
* The status endpoint is answered "configured" for the same reason, or the
|
||||||
|
* button stays disabled on a server without keys.
|
||||||
|
*/
|
||||||
|
|
||||||
|
type Failure = "worker" | "subscribe" | "none";
|
||||||
|
|
||||||
|
async function stubPush(page: Page, failure: Failure): Promise<void> {
|
||||||
|
await page.addInitScript((failAt: Failure) => {
|
||||||
|
const subscription = {
|
||||||
|
toJSON: () => ({
|
||||||
|
endpoint: "https://push.example.test/device",
|
||||||
|
keys: { p256dh: "p256dh", auth: "auth" },
|
||||||
|
}),
|
||||||
|
};
|
||||||
|
|
||||||
|
const registration = {
|
||||||
|
pushManager: {
|
||||||
|
getSubscription: async () => null,
|
||||||
|
subscribe: async () => {
|
||||||
|
if ("subscribe" === failAt) {
|
||||||
|
throw new DOMException("Registration failed - push service error", "AbortError");
|
||||||
|
}
|
||||||
|
|
||||||
|
return subscription;
|
||||||
|
},
|
||||||
|
},
|
||||||
|
};
|
||||||
|
|
||||||
|
Object.defineProperty(navigator, "serviceWorker", {
|
||||||
|
configurable: true,
|
||||||
|
value: {
|
||||||
|
register: async () => {
|
||||||
|
if ("worker" === failAt) {
|
||||||
|
throw new DOMException("The script has an unsupported MIME type", "SecurityError");
|
||||||
|
}
|
||||||
|
|
||||||
|
return registration;
|
||||||
|
},
|
||||||
|
ready: Promise.resolve(registration),
|
||||||
|
getRegistration: async () => registration,
|
||||||
|
},
|
||||||
|
});
|
||||||
|
|
||||||
|
Notification.requestPermission = async () => "granted";
|
||||||
|
}, failure);
|
||||||
|
|
||||||
|
await page.route("**/settings/push/status/**", (route) =>
|
||||||
|
route.fulfill({ json: { configured: true, registered: false, verified: false } }),
|
||||||
|
);
|
||||||
|
}
|
||||||
|
|
||||||
|
async function enable(page: Page) {
|
||||||
|
await page.goto("/settings?section=notifications");
|
||||||
|
|
||||||
|
const toggle = page.getByRole("button", { name: "Enable notifications" });
|
||||||
|
await expect(toggle).toBeEnabled();
|
||||||
|
await toggle.click();
|
||||||
|
|
||||||
|
return page.locator('[data-settings--web-push-target="state"]');
|
||||||
|
}
|
||||||
|
|
||||||
|
test.describe("enabling notifications fails with a reason", () => {
|
||||||
|
test("a service worker that will not register is named, with its error", async ({ page }) => {
|
||||||
|
await stubPush(page, "worker");
|
||||||
|
|
||||||
|
const state = await enable(page);
|
||||||
|
|
||||||
|
await expect(state).toHaveText(/background worker did not start \(SecurityError\)/);
|
||||||
|
await expect(state).toHaveAttribute("data-push-state", "error");
|
||||||
|
});
|
||||||
|
|
||||||
|
test("a push service that refuses the device is named, with Brave's switch", async ({ page }) => {
|
||||||
|
await stubPush(page, "subscribe");
|
||||||
|
|
||||||
|
const state = await enable(page);
|
||||||
|
|
||||||
|
await expect(state).toHaveText(/push service gave this device no address \(AbortError\)/);
|
||||||
|
await expect(state).toHaveText(/Use Google services for push messaging/);
|
||||||
|
});
|
||||||
|
|
||||||
|
test("a server that refuses the subscription is named, with its status", async ({ page }) => {
|
||||||
|
await stubPush(page, "none");
|
||||||
|
await page.route("**/settings/push/subscribe", (route) =>
|
||||||
|
route.fulfill({ status: 403, json: { error: "Invalid CSRF token." } }),
|
||||||
|
);
|
||||||
|
|
||||||
|
const state = await enable(page);
|
||||||
|
|
||||||
|
await expect(state).toHaveText(/server turned this device down \(HTTP 403\)/);
|
||||||
|
});
|
||||||
|
});
|
||||||
@@ -3883,6 +3883,9 @@ settings:
|
|||||||
webpush: 'Web Push'
|
webpush: 'Web Push'
|
||||||
unverified: 'Nicht bestätigt'
|
unverified: 'Nicht bestätigt'
|
||||||
verified: 'Bestätigt'
|
verified: 'Bestätigt'
|
||||||
|
error_server: 'Der Server hat dieses Gerät abgelehnt (%error%). Versuch es erneut; passiert es wieder, sag deiner Administration Bescheid.'
|
||||||
|
error_subscribe: 'Der Push-Dienst des Browsers hat diesem Gerät keine Adresse gegeben (%error%). In Brave schalte vorher „Google-Dienste für Push-Nachrichten verwenden“ ein.'
|
||||||
|
error_worker: 'Der Hintergrunddienst von plMail ist nicht gestartet (%error%). Versuch es erneut; passiert es wieder, sag deiner Administration Bescheid.'
|
||||||
heading: Benachrichtigungen
|
heading: Benachrichtigungen
|
||||||
ios_hint: >-
|
ios_hint: >-
|
||||||
Auf iPhone und iPad funktionieren Benachrichtigungen erst, wenn
|
Auf iPhone und iPad funktionieren Benachrichtigungen erst, wenn
|
||||||
|
|||||||
@@ -3869,6 +3869,9 @@ settings:
|
|||||||
webpush: 'Web Push'
|
webpush: 'Web Push'
|
||||||
unverified: 'Not verified'
|
unverified: 'Not verified'
|
||||||
verified: 'Verified'
|
verified: 'Verified'
|
||||||
|
error_server: 'The server turned this device down (%error%). Try again, and if it keeps happening, tell your administrator.'
|
||||||
|
error_subscribe: 'The browser''s push service gave this device no address (%error%). In Brave, turn on "Use Google services for push messaging" first.'
|
||||||
|
error_worker: 'plMail''s background worker did not start (%error%). Try again, and if it keeps happening, tell your administrator.'
|
||||||
heading: Notifications
|
heading: Notifications
|
||||||
ios_hint: >-
|
ios_hint: >-
|
||||||
On iPhone and iPad, notifications only work once plMail is added to
|
On iPhone and iPad, notifications only work once plMail is added to
|
||||||
|
|||||||
@@ -3670,6 +3670,9 @@ settings:
|
|||||||
webpush: "Web Push"
|
webpush: "Web Push"
|
||||||
unverified: "Not vouched for"
|
unverified: "Not vouched for"
|
||||||
verified: "Vouched for"
|
verified: "Vouched for"
|
||||||
|
error_server: 'Th'' server turned this device away (%error%). Try again, an'' if it keeps happenin'', tell yer quartermaster.'
|
||||||
|
error_subscribe: 'Th'' browser''s push service gave this device no berth (%error%). In Brave, turn on "Use Google services for push messaging" first.'
|
||||||
|
error_worker: 'plMail''s below-decks hand never woke (%error%). Try again, an'' if it keeps happenin'', tell yer quartermaster.'
|
||||||
heading: Word from the crow's nest
|
heading: Word from the crow's nest
|
||||||
ios_hint: >-
|
ios_hint: >-
|
||||||
On iPhone and iPad, the lookout only calls once plMail be added to
|
On iPhone and iPad, the lookout only calls once plMail be added to
|
||||||
|
|||||||
@@ -3919,6 +3919,9 @@ settings:
|
|||||||
webpush: 'Web Push'
|
webpush: 'Web Push'
|
||||||
unverified: 'Non vérifié'
|
unverified: 'Non vérifié'
|
||||||
verified: 'Vérifié'
|
verified: 'Vérifié'
|
||||||
|
error_server: 'Le serveur a refusé cet appareil (%error%). Réessayez ; si cela se reproduit, prévenez votre administrateur.'
|
||||||
|
error_subscribe: 'Le service push du navigateur n’a attribué aucune adresse à cet appareil (%error%). Dans Brave, activez d’abord « Utiliser les services Google pour les messages push ».'
|
||||||
|
error_worker: 'Le service d’arrière-plan de plMail n’a pas démarré (%error%). Réessayez ; si cela se reproduit, prévenez votre administrateur.'
|
||||||
heading: Notifications
|
heading: Notifications
|
||||||
ios_hint: >-
|
ios_hint: >-
|
||||||
Sur iPhone et iPad, les notifications ne fonctionnent qu’une fois plMail
|
Sur iPhone et iPad, les notifications ne fonctionnent qu’une fois plMail
|
||||||
|
|||||||
Reference in new issue
Block a user