From 9ea1b5e07b8ed2fc871cf1f27b652be67143fc80 Mon Sep 17 00:00:00 2001 From: Claude Date: Thu, 30 Jul 2026 21:21:56 -0500 Subject: [PATCH] Stop eight dashboard views reporting success for requests the server refused MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Each of these views carries its own copy of a fetch helper ending in `.then(r => r.json())`. A 403, 404 or 500 body resolves as an ordinary value, so the surrounding try/catch is unreachable and every handler treats the failure as success. The shared client in api.js has always thrown on !res.ok; these local copies never did. Two concrete consequences, both of which tell the operator something untrue: - The layout editor renders a Delete button on built-in templates for everyone. The server returns 403. The handler shows "Layout deleted" and re-renders the list with the template still sitting there. - A rejected platform-role change in Admin shows "Role updated", and the revert that would put the dropdown back lives only in the dead catch — so the UI keeps displaying a value the server refused. The same control in Settings uses the throwing client, so the two pages disagree about whether the change happened. All eight now match the shared contract: reject on !ok with the server's own message, and treat 401 as session expiry the way api.js does. This makes previously-silent failures visible, which is the point — some of them will surface refusals that were always happening. The layout template Delete button, for instance, is now honestly reported as refused rather than falsely confirmed; whether that button should be shown at all is a separate question. Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_01Uaeo9MvzKoyXuN6ZsbhtkL --- frontend/js/views/activity.js | 15 ++++++++++++++- frontend/js/views/admin.js | 15 ++++++++++++++- frontend/js/views/kiosk.js | 15 ++++++++++++++- frontend/js/views/layout-editor.js | 15 ++++++++++++++- frontend/js/views/reports.js | 15 ++++++++++++++- frontend/js/views/schedule.js | 15 ++++++++++++++- frontend/js/views/teams.js | 15 ++++++++++++++- frontend/js/views/widgets.js | 15 ++++++++++++++- 8 files changed, 112 insertions(+), 8 deletions(-) diff --git a/frontend/js/views/activity.js b/frontend/js/views/activity.js index 715fb8c..3aa361c 100644 --- a/frontend/js/views/activity.js +++ b/frontend/js/views/activity.js @@ -2,7 +2,20 @@ import { showToast } from '../components/toast.js'; import { esc } from '../utils.js'; import { t } from '../i18n.js'; -const API = (url) => fetch('/api' + url, { headers: { Authorization: `Bearer ${localStorage.getItem('token')}` }}).then(r => r.json()); +// A refused request must reject, not resolve. +// +// This helper used to end in `.then(r => r.json())`, so a 403/404/500 body resolved as an ordinary +// value and the surrounding try/catch was unreachable — every handler took the failure for success. +// Concretely: deleting a built-in layout template showed "Layout deleted" while the server had +// returned 403 and the template was still there, and a rejected platform-role change showed "Role +// updated" while the dropdown kept displaying a value the server refused (its revert lives only in +// the dead catch). The shared client in api.js has always thrown on !res.ok; these local copies did +// not. Same contract now, including the 401 session-expiry reload. +const API = (url) => fetch('/api' + url, { headers: { Authorization: `Bearer ${localStorage.getItem('token')}` }}).then(async (r) => { + if (r.status === 401) { localStorage.removeItem('token'); window.location.reload(); throw new Error('Session expired'); } + if (!r.ok) { const e = await r.json().catch(() => ({})); throw new Error(e.error || `Request failed (${r.status})`); } + return r.json(); +}); export async function render(container) { container.innerHTML = ` diff --git a/frontend/js/views/admin.js b/frontend/js/views/admin.js index 8eee022..0647949 100644 --- a/frontend/js/views/admin.js +++ b/frontend/js/views/admin.js @@ -12,7 +12,20 @@ import { openTypeToConfirmModal } from '../components/type-to-confirm-modal.js'; import { mapMutationError } from './workspace-members.js'; const headers = () => ({ Authorization: `Bearer ${localStorage.getItem('token')}`, 'Content-Type': 'application/json' }); -const API = (url, opts = {}) => fetch('/api' + url, { headers: headers(), ...opts }).then(r => r.json()); +// A refused request must reject, not resolve. +// +// This helper used to end in `.then(r => r.json())`, so a 403/404/500 body resolved as an ordinary +// value and the surrounding try/catch was unreachable — every handler took the failure for success. +// Concretely: deleting a built-in layout template showed "Layout deleted" while the server had +// returned 403 and the template was still there, and a rejected platform-role change showed "Role +// updated" while the dropdown kept displaying a value the server refused (its revert lives only in +// the dead catch). The shared client in api.js has always thrown on !res.ok; these local copies did +// not. Same contract now, including the 401 session-expiry reload. +const API = (url, opts = {}) => fetch('/api' + url, { headers: headers(), ...opts }).then(async (r) => { + if (r.status === 401) { localStorage.removeItem('token'); window.location.reload(); throw new Error('Session expired'); } + if (!r.ok) { const e = await r.json().catch(() => ({})); throw new Error(e.error || `Request failed (${r.status})`); } + return r.json(); +}); // #14: the platform user-management dropdown manages users.role (the // PLATFORM-level role) only - workspace/org roles are managed in the members diff --git a/frontend/js/views/kiosk.js b/frontend/js/views/kiosk.js index 6b6c684..acbec17 100644 --- a/frontend/js/views/kiosk.js +++ b/frontend/js/views/kiosk.js @@ -2,7 +2,20 @@ import { showToast } from '../components/toast.js'; import { t } from '../i18n.js'; import { esc } from '../utils.js'; -const API = (url, opts = {}) => fetch('/api' + url, { headers: { 'Content-Type': 'application/json', Authorization: `Bearer ${localStorage.getItem('token')}`, ...opts.headers }, ...opts }).then(r => r.json()); +// A refused request must reject, not resolve. +// +// This helper used to end in `.then(r => r.json())`, so a 403/404/500 body resolved as an ordinary +// value and the surrounding try/catch was unreachable — every handler took the failure for success. +// Concretely: deleting a built-in layout template showed "Layout deleted" while the server had +// returned 403 and the template was still there, and a rejected platform-role change showed "Role +// updated" while the dropdown kept displaying a value the server refused (its revert lives only in +// the dead catch). The shared client in api.js has always thrown on !res.ok; these local copies did +// not. Same contract now, including the 401 session-expiry reload. +const API = (url, opts = {}) => fetch('/api' + url, { headers: { 'Content-Type': 'application/json', Authorization: `Bearer ${localStorage.getItem('token')}`, ...opts.headers }, ...opts }).then(async (r) => { + if (r.status === 401) { localStorage.removeItem('token'); window.location.reload(); throw new Error('Session expired'); } + if (!r.ok) { const e = await r.json().catch(() => ({})); throw new Error(e.error || `Request failed (${r.status})`); } + return r.json(); +}); export async function render(container) { const hash = window.location.hash; diff --git a/frontend/js/views/layout-editor.js b/frontend/js/views/layout-editor.js index 07ca8a5..2279ca9 100644 --- a/frontend/js/views/layout-editor.js +++ b/frontend/js/views/layout-editor.js @@ -3,7 +3,20 @@ import { showToast } from '../components/toast.js'; import { t, tn } from '../i18n.js'; import { esc } from '../utils.js'; -const API = (url, opts = {}) => fetch('/api' + url, { headers: { 'Content-Type': 'application/json', Authorization: `Bearer ${localStorage.getItem('token')}`, ...opts.headers }, ...opts }).then(r => r.json()); +// A refused request must reject, not resolve. +// +// This helper used to end in `.then(r => r.json())`, so a 403/404/500 body resolved as an ordinary +// value and the surrounding try/catch was unreachable — every handler took the failure for success. +// Concretely: deleting a built-in layout template showed "Layout deleted" while the server had +// returned 403 and the template was still there, and a rejected platform-role change showed "Role +// updated" while the dropdown kept displaying a value the server refused (its revert lives only in +// the dead catch). The shared client in api.js has always thrown on !res.ok; these local copies did +// not. Same contract now, including the 401 session-expiry reload. +const API = (url, opts = {}) => fetch('/api' + url, { headers: { 'Content-Type': 'application/json', Authorization: `Bearer ${localStorage.getItem('token')}`, ...opts.headers }, ...opts }).then(async (r) => { + if (r.status === 401) { localStorage.removeItem('token'); window.location.reload(); throw new Error('Session expired'); } + if (!r.ok) { const e = await r.json().catch(() => ({})); throw new Error(e.error || `Request failed (${r.status})`); } + return r.json(); +}); export async function render(container) { const hash = window.location.hash; diff --git a/frontend/js/views/reports.js b/frontend/js/views/reports.js index d5ac3cd..5c7ed20 100644 --- a/frontend/js/views/reports.js +++ b/frontend/js/views/reports.js @@ -3,7 +3,20 @@ import { showToast } from '../components/toast.js'; import { esc } from '../utils.js'; import { t } from '../i18n.js'; -const API = (url, opts = {}) => fetch('/api' + url, { headers: { Authorization: `Bearer ${localStorage.getItem('token')}`, ...opts.headers }, ...opts }).then(r => r.json()); +// A refused request must reject, not resolve. +// +// This helper used to end in `.then(r => r.json())`, so a 403/404/500 body resolved as an ordinary +// value and the surrounding try/catch was unreachable — every handler took the failure for success. +// Concretely: deleting a built-in layout template showed "Layout deleted" while the server had +// returned 403 and the template was still there, and a rejected platform-role change showed "Role +// updated" while the dropdown kept displaying a value the server refused (its revert lives only in +// the dead catch). The shared client in api.js has always thrown on !res.ok; these local copies did +// not. Same contract now, including the 401 session-expiry reload. +const API = (url, opts = {}) => fetch('/api' + url, { headers: { Authorization: `Bearer ${localStorage.getItem('token')}`, ...opts.headers }, ...opts }).then(async (r) => { + if (r.status === 401) { localStorage.removeItem('token'); window.location.reload(); throw new Error('Session expired'); } + if (!r.ok) { const e = await r.json().catch(() => ({})); throw new Error(e.error || `Request failed (${r.status})`); } + return r.json(); +}); export async function render(container) { const devices = await api.getDevices(); diff --git a/frontend/js/views/schedule.js b/frontend/js/views/schedule.js index 62bb7da..b241cad 100644 --- a/frontend/js/views/schedule.js +++ b/frontend/js/views/schedule.js @@ -8,7 +8,20 @@ import { dragArmMode, LONG_PRESS_MS, DEFAULT_NEW_MIN, } from '../lib/schedule-grid.js'; -const API = (url, opts = {}) => fetch('/api' + url, { headers: { 'Content-Type': 'application/json', Authorization: `Bearer ${localStorage.getItem('token')}`, ...opts.headers }, ...opts }).then(r => r.json()); +// A refused request must reject, not resolve. +// +// This helper used to end in `.then(r => r.json())`, so a 403/404/500 body resolved as an ordinary +// value and the surrounding try/catch was unreachable — every handler took the failure for success. +// Concretely: deleting a built-in layout template showed "Layout deleted" while the server had +// returned 403 and the template was still there, and a rejected platform-role change showed "Role +// updated" while the dropdown kept displaying a value the server refused (its revert lives only in +// the dead catch). The shared client in api.js has always thrown on !res.ok; these local copies did +// not. Same contract now, including the 401 session-expiry reload. +const API = (url, opts = {}) => fetch('/api' + url, { headers: { 'Content-Type': 'application/json', Authorization: `Bearer ${localStorage.getItem('token')}`, ...opts.headers }, ...opts }).then(async (r) => { + if (r.status === 401) { localStorage.removeItem('token'); window.location.reload(); throw new Error('Session expired'); } + if (!r.ok) { const e = await r.json().catch(() => ({})); throw new Error(e.error || `Request failed (${r.status})`); } + return r.json(); +}); // Teardown registered during render (resize listener, etc). Declared here rather than beside // cleanup() so it is initialised before any render can push to it. diff --git a/frontend/js/views/teams.js b/frontend/js/views/teams.js index ca1b7d2..ff34040 100644 --- a/frontend/js/views/teams.js +++ b/frontend/js/views/teams.js @@ -3,7 +3,20 @@ import { showToast } from '../components/toast.js'; import { t, tn } from '../i18n.js'; import { esc } from '../utils.js'; -const API = (url, opts = {}) => fetch('/api' + url, { headers: { 'Content-Type': 'application/json', Authorization: `Bearer ${localStorage.getItem('token')}`, ...opts.headers }, ...opts }).then(r => r.json()); +// A refused request must reject, not resolve. +// +// This helper used to end in `.then(r => r.json())`, so a 403/404/500 body resolved as an ordinary +// value and the surrounding try/catch was unreachable — every handler took the failure for success. +// Concretely: deleting a built-in layout template showed "Layout deleted" while the server had +// returned 403 and the template was still there, and a rejected platform-role change showed "Role +// updated" while the dropdown kept displaying a value the server refused (its revert lives only in +// the dead catch). The shared client in api.js has always thrown on !res.ok; these local copies did +// not. Same contract now, including the 401 session-expiry reload. +const API = (url, opts = {}) => fetch('/api' + url, { headers: { 'Content-Type': 'application/json', Authorization: `Bearer ${localStorage.getItem('token')}`, ...opts.headers }, ...opts }).then(async (r) => { + if (r.status === 401) { localStorage.removeItem('token'); window.location.reload(); throw new Error('Session expired'); } + if (!r.ok) { const e = await r.json().catch(() => ({})); throw new Error(e.error || `Request failed (${r.status})`); } + return r.json(); +}); export async function render(container) { const hash = window.location.hash; diff --git a/frontend/js/views/widgets.js b/frontend/js/views/widgets.js index a5c815c..edda8a2 100644 --- a/frontend/js/views/widgets.js +++ b/frontend/js/views/widgets.js @@ -2,7 +2,20 @@ import { showToast } from '../components/toast.js'; import { t } from '../i18n.js'; import { hydrateAuthImages } from '../utils.js'; -const API = (url, opts = {}) => fetch('/api' + url, { headers: { 'Content-Type': 'application/json', Authorization: `Bearer ${localStorage.getItem('token')}`, ...opts.headers }, ...opts }).then(r => r.json()); +// A refused request must reject, not resolve. +// +// This helper used to end in `.then(r => r.json())`, so a 403/404/500 body resolved as an ordinary +// value and the surrounding try/catch was unreachable — every handler took the failure for success. +// Concretely: deleting a built-in layout template showed "Layout deleted" while the server had +// returned 403 and the template was still there, and a rejected platform-role change showed "Role +// updated" while the dropdown kept displaying a value the server refused (its revert lives only in +// the dead catch). The shared client in api.js has always thrown on !res.ok; these local copies did +// not. Same contract now, including the 401 session-expiry reload. +const API = (url, opts = {}) => fetch('/api' + url, { headers: { 'Content-Type': 'application/json', Authorization: `Bearer ${localStorage.getItem('token')}`, ...opts.headers }, ...opts }).then(async (r) => { + if (r.status === 401) { localStorage.removeItem('token'); window.location.reload(); throw new Error('Session expired'); } + if (!r.ok) { const e = await r.json().catch(() => ({})); throw new Error(e.error || `Request failed (${r.status})`); } + return r.json(); +}); // Widget type ids only — name + desc are looked up via t() so they switch // language with the rest of the UI.