From 40f37fea81fc660559a135977a34655473659773 Mon Sep 17 00:00:00 2001 From: Alex Sorafumo Date: Wed, 30 Sep 2026 11:02:28 +1000 Subject: [PATCH 1/9] fix: show real API error messages in toasts ts-client rejects with the raw Response on a non-OK status. The ad-hoc `JSON.stringify(err.response || err.message || err)` formatting showed "{}" or "[object Response]" instead of the reason. Move describeError to common/errors.ts and use it at every site. Add readError, which also reads the body of an unread Response, and use it where the API body carries the useful message (settings, bulk and form saves). Co-Authored-By: Claude Opus 5.5 (1M context) --- src/app/admin/broker-form.component.ts | 5 +- src/app/admin/brokers.component.ts | 5 +- src/app/admin/build-list.component.ts | 8 +- .../cluster-task-list.component.ts | 5 +- src/app/admin/database-details.component.ts | 18 +---- src/app/admin/details.component.ts | 7 +- src/app/admin/edge.component.ts | 5 +- src/app/admin/extensions.component.ts | 3 +- .../signage-plugin-import-modal.component.ts | 3 +- .../signage-plugin-modal.component.ts | 5 +- .../signage-plugins.component.ts | 5 +- src/app/admin/staff-api.component.ts | 7 +- src/app/common/errors.ts | 73 +++++++++++++++++++ src/app/common/item.service.ts | 6 +- src/app/domains/domain-form.component.ts | 7 +- src/app/domains/domain-state.service.ts | 13 ++-- src/app/drivers/driver-form.component.ts | 7 +- src/app/drivers/driver-state.service.ts | 5 +- .../overlays/auth-source-modal.component.ts | 7 +- .../bulk-item-modal/status-list.component.ts | 25 +------ src/app/overlays/confirm-modal.component.ts | 25 +------ src/app/overlays/duplicate-modal.component.ts | 3 +- .../overlays/view-module-state.component.ts | 3 +- .../repositories-state.service.ts | 20 ++--- .../repositories/repository-form.component.ts | 7 +- src/app/systems/system-form.component.ts | 7 +- src/app/systems/system-modules.component.ts | 5 +- src/app/systems/system-state.service.ts | 63 ++++++---------- src/app/triggers/trigger-form.component.ts | 7 +- src/app/triggers/trigger-state.service.ts | 21 ++---- src/app/ui/forms/settings-form.component.ts | 13 ++-- .../forms/trigger-action-modal.component.ts | 5 +- .../trigger-condition-modal.component.ts | 3 +- src/app/ui/metadata-display.component.ts | 19 ++--- src/app/zones/zone-form.component.ts | 7 +- src/app/zones/zones-state.service.ts | 7 +- src/tests/common/errors.spec.ts | 58 +++++++++++++++ .../overlays/confirm-modal.component.spec.ts | 30 -------- 38 files changed, 255 insertions(+), 267 deletions(-) create mode 100644 src/app/common/errors.ts create mode 100644 src/tests/common/errors.spec.ts diff --git a/src/app/admin/broker-form.component.ts b/src/app/admin/broker-form.component.ts index a0d6d9838..f439be5cc 100644 --- a/src/app/admin/broker-form.component.ts +++ b/src/app/admin/broker-form.component.ts @@ -22,6 +22,7 @@ import { updateBroker, } from '@placeos/ts-client'; import { AsyncHandler } from '../common/async-handler.class'; +import { readError } from '../common/errors'; import { addSignalChipItem, getInvalidSignalFields } from '../common/forms'; import { HotkeysService } from '../common/hotkeys.service'; import { i18n } from '../common/locale.service'; @@ -429,9 +430,7 @@ export class BrokerFormComponent extends AsyncHandler implements OnInit { this._dialog_ref.disableClose = false; notifyError( i18n(`${this._name}.SAVE_ERROR`, { - error: JSON.stringify( - (await err.text?.()) || err.message || err, - ), + error: await readError(err), }), ); return null; diff --git a/src/app/admin/brokers.component.ts b/src/app/admin/brokers.component.ts index 234d0f34c..cb0092458 100644 --- a/src/app/admin/brokers.component.ts +++ b/src/app/admin/brokers.component.ts @@ -14,6 +14,7 @@ import { } from '@placeos/ts-client'; import { AsyncHandler } from '../common/async-handler.class'; +import { describeError } from '../common/errors'; import { notifyError, notifySuccess } from '../common/notifications'; import { FormModalComponent } from '../common/types'; import { openConfirmModal } from '../overlays/confirm-modal.component'; @@ -257,9 +258,7 @@ export class AdminBrokersComponent extends AsyncHandler implements OnInit { details.close(); if (err) return notifyError( - `Error deleting broker. Error: ${JSON.stringify( - err.response || err.message || err, - )}`, + `Error deleting broker. Error: ${describeError(err)}`, ); notifySuccess(`Successfully deleted broker "${item.name}".`); this.loadBrokers(); diff --git a/src/app/admin/build-list.component.ts b/src/app/admin/build-list.component.ts index 3b7f686d4..7d0082f32 100644 --- a/src/app/admin/build-list.component.ts +++ b/src/app/admin/build-list.component.ts @@ -5,6 +5,7 @@ import { MatProgressBarModule } from '@angular/material/progress-bar'; import { MatTooltipModule } from '@angular/material/tooltip'; import { del, get } from '@placeos/ts-client'; import { toQueryString } from '../common/api'; +import { describeError } from '../common/errors'; import { escapeHtml } from '../common/general'; import { i18n } from '../common/locale.service'; import { notifyError, notifySuccess } from '../common/notifications'; @@ -170,12 +171,7 @@ export class PlaceBuildListComponent implements OnInit { if (err) return notifyError( i18n('ADMIN.BUILD_LIST_REMOVE_ERROR', { - error: - (err as { statusText?: string; message?: string }) - .statusText || - (err as { statusText?: string; message?: string }) - .message || - err, + error: describeError(err), }), ); this.last_change.set(null); diff --git a/src/app/admin/cluster-details/cluster-task-list.component.ts b/src/app/admin/cluster-details/cluster-task-list.component.ts index e23433cbc..b689bbf71 100644 --- a/src/app/admin/cluster-details/cluster-task-list.component.ts +++ b/src/app/admin/cluster-details/cluster-task-list.component.ts @@ -23,6 +23,7 @@ import { MatProgressBarModule } from '@angular/material/progress-bar'; import { MatTooltipModule } from '@angular/material/tooltip'; import { ActivatedRoute, RouterModule } from '@angular/router'; import { AsyncHandler } from '../../common/async-handler.class'; +import { describeError } from '../../common/errors'; import { i18n } from '../../common/locale.service'; import { notifyError } from '../../common/notifications'; import { signalFromSubscribable } from '../../common/signals'; @@ -274,9 +275,7 @@ export class PlaceClusterTaskListComponent this.killing.set(null); notifyError( i18n('ADMIN.CLUSTER_PROCESS_KILL_ERROR', { - error: JSON.stringify( - err.response || err.message || err, - ), + error: describeError(err), }), ); ref.close(); diff --git a/src/app/admin/database-details.component.ts b/src/app/admin/database-details.component.ts index 765ca4940..e3639de5e 100644 --- a/src/app/admin/database-details.component.ts +++ b/src/app/admin/database-details.component.ts @@ -4,13 +4,13 @@ import { addZone, PlaceZone, queryZones, showZone } from '@placeos/ts-client'; import { MatRippleModule } from '@angular/material/core'; import { MatDialog, MatDialogModule } from '@angular/material/dialog'; import { MatProgressSpinnerModule } from '@angular/material/progress-spinner'; +import { describeError } from '../common/errors'; import { csvToJson, downloadFile, jsonToCsv } from '../common/general'; import { notifyError, notifySuccess, notifyWarn, } from '../common/notifications'; -import { describeError } from '../overlays/confirm-modal.component'; import { ZoneTreeExportModalComponent } from './zone-tree-export-modal.component'; type ZoneTreeExportItem = Record & { @@ -200,13 +200,7 @@ export class PlaceDatabaseDetailsComponent { await this.importZoneTree(zones); } catch (err) { notifyError( - `Error importing zone tree. Error: ${JSON.stringify( - (err as { response?: unknown; message?: unknown }) - .response || - (err as { response?: unknown; message?: unknown }) - .message || - err, - )}`, + `Error importing zone tree. Error: ${describeError(err)}`, ); } this.importing_zones.set(false); @@ -227,13 +221,7 @@ export class PlaceDatabaseDetailsComponent { notifySuccess(`Exported ${zones.length} zones.`); } catch (err) { notifyError( - `Error exporting zone tree. Error: ${JSON.stringify( - (err as { response?: unknown; message?: unknown }) - .response || - (err as { response?: unknown; message?: unknown }) - .message || - err, - )}`, + `Error exporting zone tree. Error: ${describeError(err)}`, ); } this.exporting_zones.set(false); diff --git a/src/app/admin/details.component.ts b/src/app/admin/details.component.ts index 9456fea3d..9b5bc3643 100644 --- a/src/app/admin/details.component.ts +++ b/src/app/admin/details.component.ts @@ -13,6 +13,7 @@ import { BackofficeUsersService } from '../users/users.service'; import { DatePipe, SlicePipe } from '@angular/common'; import { format } from 'date-fns'; +import { describeError } from '../common/errors'; import { copyToClipboard } from '../common/general'; import { i18n } from '../common/locale.service'; import { TranslatePipe } from '../ui/translate.pipe'; @@ -253,9 +254,7 @@ export class PlaceDetailsComponent extends AsyncHandler implements OnInit { (err) => notifyError( i18n('ADMIN.BACKEND_SERVICES_ERROR', { - error: JSON.stringify( - err.response || err.message || err, - ), + error: describeError(err), }), ), ); @@ -268,7 +267,7 @@ export class PlaceDetailsComponent extends AsyncHandler implements OnInit { ).catch((err) => { notifyError( i18n('ADMIN.BACKEND_SERVICES_ERROR', { - error: JSON.stringify(err.response || err.message || err), + error: describeError(err), }), ); throw err; diff --git a/src/app/admin/edge.component.ts b/src/app/admin/edge.component.ts index db09326bb..043d91a75 100644 --- a/src/app/admin/edge.component.ts +++ b/src/app/admin/edge.component.ts @@ -11,6 +11,7 @@ import { removeEdge, retrieveEdgeToken, } from '@placeos/ts-client'; +import { describeError } from '../common/errors'; import { copyToClipboard, escapeHtml } from '../common/general'; import { notifyError, @@ -237,9 +238,7 @@ export class PlaceEdgeComponent implements OnInit { details.close(); if (err) return notifyError( - `Error removing edge. Error: ${ - err.statusText || err.message || err - }`, + `Error removing edge. Error: ${describeError(err)}`, ); sessionStorage.removeItem('BACKOFFICE.last_edge'); this.last_change.set(null); diff --git a/src/app/admin/extensions.component.ts b/src/app/admin/extensions.component.ts index 688dd9b2a..3bda94579 100644 --- a/src/app/admin/extensions.component.ts +++ b/src/app/admin/extensions.component.ts @@ -7,6 +7,7 @@ import { MatProgressBarModule } from '@angular/material/progress-bar'; import { MatSelectModule } from '@angular/material/select'; import { MatTooltipModule } from '@angular/material/tooltip'; import { PlaceDomain, updateDomain } from '@placeos/ts-client'; +import { describeError } from '../common/errors'; import { escapeHtml } from '../common/general'; import { notifyError } from '../common/notifications'; import { waitForEvent } from '../common/signals'; @@ -275,7 +276,7 @@ export class PlaceExtensionsComponent implements OnInit { let ext_list = this.extensions(); ext_list = ext_list.filter((i) => !isSameExtension(i, item)); await this.updateDomain(ext_list).catch((e) => - notifyError(`Error removing extension: ${e}`), + notifyError(`Error removing extension: ${describeError(e)}`), ); ref.componentInstance?.loading.set(''); ref.close(); diff --git a/src/app/admin/signage-plugins/signage-plugin-import-modal.component.ts b/src/app/admin/signage-plugins/signage-plugin-import-modal.component.ts index c821a527a..e7fa54a7c 100644 --- a/src/app/admin/signage-plugins/signage-plugin-import-modal.component.ts +++ b/src/app/admin/signage-plugins/signage-plugin-import-modal.component.ts @@ -22,6 +22,7 @@ import { } from '@placeos/ts-client'; import { AsyncHandler } from '../../common/async-handler.class'; +import { describeError } from '../../common/errors'; import { i18n } from '../../common/locale.service'; import { notifyError } from '../../common/notifications'; import { IconComponent } from '../../ui/icon.component'; @@ -274,7 +275,7 @@ export class SignagePluginImportModalComponent private _notifyError(err: { response?: unknown; message?: string }) { notifyError( i18n('ADMIN.SIGNAGE_PLUGINS_IMPORT_ERROR', { - error: JSON.stringify(err.response || err.message || err), + error: describeError(err), }), ); } diff --git a/src/app/admin/signage-plugins/signage-plugin-modal.component.ts b/src/app/admin/signage-plugins/signage-plugin-modal.component.ts index d6dec179f..4926687da 100644 --- a/src/app/admin/signage-plugins/signage-plugin-modal.component.ts +++ b/src/app/admin/signage-plugins/signage-plugin-modal.component.ts @@ -18,6 +18,7 @@ import { MatSelectModule } from '@angular/material/select'; import { SignagePlugin } from '@placeos/ts-client'; import { AsyncHandler } from '../../common/async-handler.class'; +import { readError } from '../../common/errors'; import { getInvalidSignalFields } from '../../common/forms'; import { HotkeysService } from '../../common/hotkeys.service'; import { i18n } from '../../common/locale.service'; @@ -390,9 +391,7 @@ export class SignagePluginModalComponent this._dialog_ref.disableClose = false; notifyError( i18n('ADMIN.SIGNAGE_PLUGINS_SAVE_ERROR', { - error: JSON.stringify( - err.response || err.message || err, - ), + error: await readError(err), }), ); } diff --git a/src/app/admin/signage-plugins/signage-plugins.component.ts b/src/app/admin/signage-plugins/signage-plugins.component.ts index 1edcdd915..9a14fe371 100644 --- a/src/app/admin/signage-plugins/signage-plugins.component.ts +++ b/src/app/admin/signage-plugins/signage-plugins.component.ts @@ -12,6 +12,7 @@ import { } from '@placeos/ts-client'; import { AsyncHandler } from '../../common/async-handler.class'; +import { describeError } from '../../common/errors'; import { i18n } from '../../common/locale.service'; import { notifyError, notifySuccess } from '../../common/notifications'; import { openConfirmModal } from '../../overlays/confirm-modal.component'; @@ -289,7 +290,7 @@ export class AdminSignagePluginsComponent if (err) return notifyError( i18n('ADMIN.SIGNAGE_PLUGINS_REMOVE_ERROR', { - error: JSON.stringify(err.response || err.message || err), + error: describeError(err), }), ); notifySuccess(i18n('ADMIN.SIGNAGE_PLUGINS_REMOVE_SUCCESS')); @@ -304,7 +305,7 @@ export class AdminSignagePluginsComponent } catch (err) { notifyError( i18n('ADMIN.SIGNAGE_PLUGINS_LOAD_ERROR', { - error: JSON.stringify(err.response || err.message || err), + error: describeError(err), }), ); } finally { diff --git a/src/app/admin/staff-api.component.ts b/src/app/admin/staff-api.component.ts index dadccc32b..6bd6b6c1d 100644 --- a/src/app/admin/staff-api.component.ts +++ b/src/app/admin/staff-api.component.ts @@ -8,6 +8,7 @@ import { MatProgressBarModule } from '@angular/material/progress-bar'; import { MatSelectModule } from '@angular/material/select'; import { MatTooltipModule } from '@angular/material/tooltip'; import { del, get, PlaceDomain } from '@placeos/ts-client'; +import { describeError } from '../common/errors'; import { escapeHtml } from '../common/general'; import { notifyError, notifySuccess } from '../common/notifications'; import { HashMap } from '../common/types'; @@ -258,9 +259,9 @@ export class PlaceStaffAPIComponent implements OnInit { const system = await del(`/api/staff/v1/tenants/${tenant.id}`).catch( (err) => { notifyError( - `Error removing module ${tenant.id} from domain. Error: ${ - err.statusText || err.message || err - }`, + `Error removing module ${tenant.id} from domain. Error: ${describeError( + err, + )}`, ); return true; }, diff --git a/src/app/common/errors.ts b/src/app/common/errors.ts new file mode 100644 index 000000000..5764c76dd --- /dev/null +++ b/src/app/common/errors.ts @@ -0,0 +1,73 @@ +/** Longest body text to show in a notification */ +const MAX_BODY_LENGTH = 300; + +/** + * Readable text for a caught error. Use it wherever an error is shown to the + * user. + * + * ts-client throws the raw `Response` for any non-OK status. Interpolating it + * yields "[object Response]" and `JSON.stringify` yields "{}", so read the + * status from it instead. Use `readError` when the body text is wanted too. + */ +export function describeError(error: unknown): string { + if (!error) return 'Unknown error'; + if (typeof error === 'string') return error; + if (typeof error !== 'object') return String(error); + if (typeof Response !== 'undefined' && error instanceof Response) { + return `${error.status} ${error.statusText || 'request failed'}`.trim(); + } + const { message, status, statusText } = error as { + message?: unknown; + status?: unknown; + statusText?: unknown; + }; + if (typeof message === 'string' && message) return message; + if (typeof status === 'number' && status) { + return `${status} ${statusText || 'request failed'}`.trim(); + } + return 'Unknown error'; +} + +/** + * Like `describeError`, but also reads the body of a `Response` that is not + * read yet, so the message from the API reaches the user. + * + * Reads a clone, so other handlers can still read the original body. + */ +export async function readError(error: unknown): Promise { + const summary = describeError(error); + if ( + typeof Response === 'undefined' || + !(error instanceof Response) || + error.bodyUsed + ) { + return summary; + } + const body = await error + .clone() + .text() + .catch(() => ''); + const detail = bodyMessage(body); + return detail ? `${summary}: ${detail}` : summary; +} + +/** Pulls the useful message out of an error body, JSON or plain text */ +function bodyMessage(body: string): string { + const text = body.trim(); + if (!text) return ''; + try { + const json = JSON.parse(text) as { + message?: unknown; + error?: unknown; + }; + const message = json?.message || json?.error; + if (typeof message === 'string' && message) { + return message.slice(0, MAX_BODY_LENGTH); + } + } catch { + // Not JSON, so show the text as is + } + // An HTML error page is noise in a notification + if (text.startsWith('<')) return ''; + return text.slice(0, MAX_BODY_LENGTH); +} diff --git a/src/app/common/item.service.ts b/src/app/common/item.service.ts index 353a288b9..43366422c 100644 --- a/src/app/common/item.service.ts +++ b/src/app/common/item.service.ts @@ -22,7 +22,6 @@ import { CONFIRM_METADATA, ConfirmModalComponent, ConfirmModalData, - describeError, } from '../overlays/confirm-modal.component'; import { DuplicateModalComponent } from '../overlays/duplicate-modal.component'; import { BackofficeUsersService } from '../users/users.service'; @@ -34,6 +33,7 @@ import { CascadeResource, runCascade, } from './cascade-delete'; +import { describeError } from './errors'; import { escapeHtml, log } from './general'; import { i18n } from './locale.service'; import { notifyError, notifySuccess } from './notifications'; @@ -516,9 +516,7 @@ export class ActiveItemService extends AsyncHandler { } notifyError( i18n(`${actions.name}.DELETE_ERROR`, { - error: JSON.stringify( - err.response || err.message || err, - ), + error: describeError(err), }), ); }); diff --git a/src/app/domains/domain-form.component.ts b/src/app/domains/domain-form.component.ts index f365041c6..00ffeda14 100644 --- a/src/app/domains/domain-form.component.ts +++ b/src/app/domains/domain-form.component.ts @@ -23,6 +23,7 @@ import { updateDomain, } from '@placeos/ts-client'; import { AsyncHandler } from '../common/async-handler.class'; +import { readError } from '../common/errors'; import { addSignalChipItem, getInvalidSignalFields, @@ -375,14 +376,12 @@ export class DomainFormComponent extends AsyncHandler implements OnInit { settings_string, encryption_level: EncryptionLevel.Support, }); - await addSettings(new_settings).catch((err) => { + await addSettings(new_settings).catch(async (err) => { this.loading.set(null); notifyError( `Error saving settings for ${ item.name || item.id - }. Error: ${JSON.stringify( - err.response || err.message || err, - )}`, + }. Error: ${await readError(err)}`, ); }); } diff --git a/src/app/domains/domain-state.service.ts b/src/app/domains/domain-state.service.ts index 9ab0eebd6..b91940a44 100644 --- a/src/app/domains/domain-state.service.ts +++ b/src/app/domains/domain-state.service.ts @@ -31,6 +31,7 @@ import { updateDomain, } from '@placeos/ts-client'; import { filter, map } from 'rxjs'; +import { describeError } from '../common/errors'; import { escapeHtml } from '../common/general'; import { ActiveItemService } from '../common/item.service'; import { i18n } from '../common/locale.service'; @@ -270,9 +271,9 @@ export class DomainStateService { details.close(); if (err) return notifyError( - `Error removing domain application. Error: ${ - err.responseText || err.message || err - }`, + `Error removing domain application. Error: ${describeError( + err, + )}`, ); notifySuccess('Successfully removed domain application.'); this._changed.set(new Date().valueOf()); @@ -324,9 +325,9 @@ export class DomainStateService { details.close(); if (err) return notifyError( - `Error removing domain auth source. Error: ${ - err.responseText || err.message || err - }`, + `Error removing domain auth source. Error: ${describeError( + err, + )}`, ); notifySuccess('Successfully removed domain auth source.'); this._changed.set(new Date().valueOf()); diff --git a/src/app/drivers/driver-form.component.ts b/src/app/drivers/driver-form.component.ts index 9e62fa054..2b2f209f8 100644 --- a/src/app/drivers/driver-form.component.ts +++ b/src/app/drivers/driver-form.component.ts @@ -47,6 +47,7 @@ import { i18n } from '../common/locale.service'; import { notifyError, notifySuccess } from '../common/notifications'; import { DialogEvent, Identity } from '../common/types'; +import { readError } from '../common/errors'; import { ItemSearchFieldComponent } from '../ui/custom-fields/item-search-field.component'; import { FullscreenModalShellComponent } from '../ui/fullscreen-modal-shell.component'; import { SettingsToggleComponent } from '../ui/settings-toggle.component'; @@ -587,14 +588,12 @@ export class DriverFormComponent extends AsyncHandler implements OnInit { settings_string, encryption_level: EncryptionLevel.Support, }); - await addSettings(new_settings).catch((err) => { + await addSettings(new_settings).catch(async (err) => { this.saving.set(null); notifyError( `Error saving settings for ${ item.name || item.id - }. Error: ${JSON.stringify( - err.response || err.message || err, - )}`, + }. Error: ${await readError(err)}`, ); }); } diff --git a/src/app/drivers/driver-state.service.ts b/src/app/drivers/driver-state.service.ts index 29143f8ee..df4db2888 100644 --- a/src/app/drivers/driver-state.service.ts +++ b/src/app/drivers/driver-state.service.ts @@ -11,6 +11,7 @@ import { removeModule, updateDriver, } from '@placeos/ts-client'; +import { describeError } from '../common/errors'; import { ActiveItemService } from '../common/item.service'; import { notifyError, notifySuccess } from '../common/notifications'; import { HashMap, Identity } from '../common/types'; @@ -198,9 +199,7 @@ export class DriverStateService { .then(() => true) .catch((err) => { notifyError( - `Error removing module ${device.id}. Error: ${ - err.statusText || err.message || err - }`, + `Error removing module ${device.id}. Error: ${describeError(err)}`, ); return false; }); diff --git a/src/app/overlays/auth-source-modal.component.ts b/src/app/overlays/auth-source-modal.component.ts index f56fc1b6a..12565cdf1 100644 --- a/src/app/overlays/auth-source-modal.component.ts +++ b/src/app/overlays/auth-source-modal.component.ts @@ -25,6 +25,7 @@ import { import { MatFormFieldModule } from '@angular/material/form-field'; import { MatSelectModule } from '@angular/material/select'; import { AsyncHandler } from '../common/async-handler.class'; +import { readError } from '../common/errors'; import { i18n } from '../common/locale.service'; import { notifyError, notifySuccess } from '../common/notifications'; import { DialogEvent, Identity } from '../common/types'; @@ -241,13 +242,11 @@ export class AuthSourceModalComponent extends AsyncHandler implements OnInit { notifySuccess(i18n('DOMAINS.AUTHENTICATION_SAVE_SUCCESS')); this._dialog.close(); }, - (err) => { + async (err) => { this.loading.set(''); notifyError( i18n('DOMAINS.AUTHENTICATION_SAVE_ERROR', { - error: JSON.stringify( - err.response || err.message || err, - ), + error: await readError(err), }), ); }, diff --git a/src/app/overlays/bulk-item-modal/status-list.component.ts b/src/app/overlays/bulk-item-modal/status-list.component.ts index 945755482..bca9f139d 100644 --- a/src/app/overlays/bulk-item-modal/status-list.component.ts +++ b/src/app/overlays/bulk-item-modal/status-list.component.ts @@ -10,6 +10,7 @@ import { MatRippleModule } from '@angular/material/core'; import { MatProgressSpinnerModule } from '@angular/material/progress-spinner'; import { MatTooltipModule } from '@angular/material/tooltip'; import { PlaceResource } from '@placeos/ts-client'; +import { readError } from '../../common/errors'; import { notifyError } from '../../common/notifications'; import { IconComponent } from '../../ui/icon.component'; import { TranslatePipe } from '../../ui/translate.pipe'; @@ -174,7 +175,7 @@ export class StatusListComponent implements OnChanges { this.completed_count.set(success_count); results[index] = saved_item; } catch (err) { - const message = this.formatError(err); + const message = `Error: ${await readError(err)}`; this.setStatus(index, message); console.error(`Failed to save item ${index}:`, err); notifyError(message); @@ -196,26 +197,4 @@ export class StatusListComponent implements OnChanges { private setStatus(index: number, value: string): void { this.status.update((status) => ({ ...status, [index]: value })); } - - private formatError(err: unknown): string { - if (err && typeof err === 'object') { - const http_err = err as Record; - const status = http_err.status || ''; - const status_text = http_err.statusText || ''; - const message = - http_err.message || - (http_err.error && - typeof http_err.error === 'object' && - (http_err.error as Record).message - ? (http_err.error as Record).message - : ''); - if (status || status_text) { - return `Error: ${status} ${status_text}`.trim(); - } - if (message) { - return `Error: ${message}`; - } - } - return `Error: ${String(err)}`; - } } diff --git a/src/app/overlays/confirm-modal.component.ts b/src/app/overlays/confirm-modal.component.ts index 0b4f1387c..474c33a50 100644 --- a/src/app/overlays/confirm-modal.component.ts +++ b/src/app/overlays/confirm-modal.component.ts @@ -21,6 +21,7 @@ import { MatRippleModule } from '@angular/material/core'; import { MatProgressSpinnerModule } from '@angular/material/progress-spinner'; import { lastValueFrom } from 'rxjs'; import { AsyncHandler } from '../common/async-handler.class'; +import { describeError } from '../common/errors'; import { i18n } from '../common/locale.service'; import { notifyInfo } from '../common/notifications'; import { waitForEvent } from '../common/signals'; @@ -105,30 +106,6 @@ export interface ConfirmModalData { close_delay?: number; } -/** - * Readable text for whatever an option's `details()` rejected with. - * - * ts-client throws the raw `Response` for any non-OK status, and interpolating - * that yields "[object Response]" — which is what the user would otherwise be - * shown as the reason they cannot continue. - */ -export function describeError(error: unknown): string { - if (!error) return 'Unknown error'; - if (typeof error === 'string') return error; - if (typeof Response !== 'undefined' && error instanceof Response) { - return `${error.status} ${error.statusText || 'request failed'}`.trim(); - } - const message = (error as Error)?.message; - if (message) return message; - const status = (error as { status?: number; statusText?: string })?.status; - if (status) { - return `${status} ${ - (error as { statusText?: string }).statusText || 'request failed' - }`.trim(); - } - return 'Unknown error'; -} - /** * Renders a receipt as tab separated rows, for pasting into a ticket or * spreadsheet. Failed rows are marked so a partial run is not mistaken for a diff --git a/src/app/overlays/duplicate-modal.component.ts b/src/app/overlays/duplicate-modal.component.ts index e006d8772..172a9788c 100644 --- a/src/app/overlays/duplicate-modal.component.ts +++ b/src/app/overlays/duplicate-modal.component.ts @@ -16,6 +16,7 @@ import { FormsModule } from '@angular/forms'; import { MatFormFieldModule } from '@angular/material/form-field'; import { MatInputModule } from '@angular/material/input'; import { MatProgressSpinnerModule } from '@angular/material/progress-spinner'; +import { describeError } from '../common/errors'; import { notifyError } from '../common/notifications'; import { DialogEvent, HashMap } from '../common/types'; import { IconComponent } from '../ui/icon.component'; @@ -227,7 +228,7 @@ export class DuplicateModalComponent { }); this.setStatus(i, 'loading'); const saved_item = await this._data.save(new_item).catch((err) => { - this.setStatus(i, `Error: ${err.message || err}`); + this.setStatus(i, `Error: ${describeError(err)}`); notifyError(this.status()[i]); }); list.push(saved_item); diff --git a/src/app/overlays/view-module-state.component.ts b/src/app/overlays/view-module-state.component.ts index 6f2c80626..87e5dd330 100644 --- a/src/app/overlays/view-module-state.component.ts +++ b/src/app/overlays/view-module-state.component.ts @@ -14,6 +14,7 @@ import { import { FormsModule } from '@angular/forms'; import { MatRippleModule } from '@angular/material/core'; import { AsyncHandler } from '../common/async-handler.class'; +import { describeError } from '../common/errors'; import { notifyError } from '../common/notifications'; import { HashMap } from '../common/types'; import { SettingsFieldComponent } from '../ui/custom-fields/settings-field.component'; @@ -199,7 +200,7 @@ export class ViewModuleStateModalComponent }); this.state.set(JSON.stringify(pre_state, undefined, 4)); } catch (err) { - notifyError(JSON.stringify(err.response || err.message || err)); + notifyError(describeError(err)); } finally { this.loading.set(false); } diff --git a/src/app/repositories/repositories-state.service.ts b/src/app/repositories/repositories-state.service.ts index e39f3e769..331004bf6 100644 --- a/src/app/repositories/repositories-state.service.ts +++ b/src/app/repositories/repositories-state.service.ts @@ -10,6 +10,7 @@ import { showRepository, } from '@placeos/ts-client'; +import { describeError } from '../common/errors'; import { ActiveItemService } from '../common/item.service'; import { i18n } from '../common/locale.service'; import { notifyError } from '../common/notifications'; @@ -46,7 +47,7 @@ export class RepositoriesStateService { try { return await listRepositoryDrivers(item.id, { limit: 2000 }); } catch (err) { - this._driver_list_error.set(this._errorMessage(err)); + this._driver_list_error.set(describeError(err)); return []; } finally { this._loading.set(false); @@ -102,7 +103,7 @@ export class RepositoriesStateService { count: 1, } as Record); } catch (err) { - this._commit_error.set(this._errorMessage(err)); + this._commit_error.set(describeError(err)); } return details[0]?.commit || 'HEAD'; }, @@ -120,11 +121,9 @@ export class RepositoriesStateService { ).catch((err) => { notifyError( i18n('REPOS.GIT_PULL_ERROR', { - error: JSON.stringify( - err.response || - err.message || - i18n('REPOS.GIT_PULL_TIMEOUT'), - ), + error: err + ? describeError(err) + : i18n('REPOS.GIT_PULL_TIMEOUT'), }), ); }); @@ -145,11 +144,4 @@ export class RepositoriesStateService { }, }); } - - private _errorMessage(err: unknown): string { - if (err instanceof Response) return `${err.status} ${err.statusText}`; - if (err instanceof Error) return err.message; - const error = err as { response?: unknown; message?: unknown }; - return JSON.stringify(error?.response || error?.message || err); - } } diff --git a/src/app/repositories/repository-form.component.ts b/src/app/repositories/repository-form.component.ts index 4b9b13a09..5a8086497 100644 --- a/src/app/repositories/repository-form.component.ts +++ b/src/app/repositories/repository-form.component.ts @@ -40,6 +40,7 @@ import { updateRepository, } from '@placeos/ts-client'; import { AsyncHandler } from '../common/async-handler.class'; +import { readError } from '../common/errors'; import { getInvalidSignalFields } from '../common/forms'; import { HotkeysService } from '../common/hotkeys.service'; import { i18n } from '../common/locale.service'; @@ -838,14 +839,12 @@ export class RepositoryFormComponent extends AsyncHandler implements OnInit { settings_string, encryption_level: EncryptionLevel.Support, }); - await addSettings(new_settings).catch((err) => { + await addSettings(new_settings).catch(async (err) => { this.saving.set(null); notifyError( `Error saving settings for ${ item.name || item.id - }. Error: ${JSON.stringify( - err.response || err.message || err, - )}`, + }. Error: ${await readError(err)}`, ); }); } diff --git a/src/app/systems/system-form.component.ts b/src/app/systems/system-form.component.ts index 6f3dc4ff5..5ad826fee 100644 --- a/src/app/systems/system-form.component.ts +++ b/src/app/systems/system-form.component.ts @@ -39,6 +39,7 @@ import { notifyError, notifySuccess } from '../common/notifications'; import { TIMEZONES_IANA } from '../common/timezones'; import { DialogEvent, Identity } from '../common/types'; +import { readError } from '../common/errors'; import { CounterComponent } from '../ui/counter.component'; import { ImageListFieldComponent } from '../ui/custom-fields/image-list-field.component'; import { ItemSearchFieldComponent } from '../ui/custom-fields/item-search-field.component'; @@ -729,14 +730,12 @@ export class SystemFormComponent extends AsyncHandler implements OnInit { settings_string, encryption_level: EncryptionLevel.Support, }); - await addSettings(new_settings).catch((err) => { + await addSettings(new_settings).catch(async (err) => { this.loading.set(null); notifyError( `Error saving settings for ${ item.name || item.id - }. Error: ${JSON.stringify( - err.response || err.message || err, - )}`, + }. Error: ${await readError(err)}`, ); }); } diff --git a/src/app/systems/system-modules.component.ts b/src/app/systems/system-modules.component.ts index e86a9b925..a14a502f1 100644 --- a/src/app/systems/system-modules.component.ts +++ b/src/app/systems/system-modules.component.ts @@ -24,6 +24,7 @@ import { showModule, } from '@placeos/ts-client'; import { AsyncHandler } from '../common/async-handler.class'; +import { describeError } from '../common/errors'; import { i18n } from '../common/locale.service'; import { notifyError, notifySuccess } from '../common/notifications'; import { AppLink, HashMap } from '../common/types'; @@ -617,9 +618,7 @@ export class SystemModulesComponent extends AsyncHandler { ), (err) => notifyError( - `Error loading module. Error: ${JSON.stringify( - err.response || err.message || err, - )}`, + `Error loading module. Error: ${describeError(err)}`, ), ); } diff --git a/src/app/systems/system-state.service.ts b/src/app/systems/system-state.service.ts index f05a38a5c..bbe2583d2 100644 --- a/src/app/systems/system-state.service.ts +++ b/src/app/systems/system-state.service.ts @@ -37,6 +37,7 @@ import { querySupportSystems as querySystems, } from '../common/support-access'; +import { describeError } from '../common/errors'; import { ActiveItemService } from '../common/item.service'; import { notifyError, notifySuccess } from '../common/notifications'; import { waitForEvent } from '../common/signals'; @@ -284,11 +285,7 @@ export class SystemStateService extends AsyncHandler { const error = await startSystem(this.active_item.id) .then(() => null) .catch((err) => { - notifyError( - `Failed to start system: ${JSON.stringify( - err.response || err.message || err, - )}`, - ); + notifyError(`Failed to start system: ${describeError(err)}`); return err; }); if (!error) { @@ -312,11 +309,7 @@ export class SystemStateService extends AsyncHandler { const error = await stopSystem(this.active_item.id) .then(() => null) .catch((err) => { - notifyError( - `Failed to stop system: ${JSON.stringify( - err.response || err.message || err, - )}`, - ); + notifyError(`Failed to stop system: ${describeError(err)}`); return err; }); if (!error) { @@ -392,7 +385,7 @@ export class SystemStateService extends AsyncHandler { `Error adding module to system "${ (system as PlaceSystem & { display_name?: string }) .display_name || system.name - }". Error: ${JSON.stringify(_e.response || _e.message || _e)}`, + }". Error: ${describeError(_e)}`, ); throw _e; }); @@ -471,9 +464,7 @@ export class SystemStateService extends AsyncHandler { }/triggers/${trigger.id}`; const trig = await put(url, details.metadata).catch((err) => { notifyError( - `Error updating trigger settings. Error: ${JSON.stringify( - err.response || err.message || err, - )}`, + `Error updating trigger settings. Error: ${describeError(err)}`, ); throw err; }); @@ -497,9 +488,9 @@ export class SystemStateService extends AsyncHandler { (err) => { details.close(); notifyError( - `Error removing trigger ${trigger.id} from system. Error: ${ - err.statusText || err.message || err - }`, + `Error removing trigger ${trigger.id} from system. Error: ${describeError( + err, + )}`, ); throw err; }, @@ -524,9 +515,7 @@ export class SystemStateService extends AsyncHandler { modules: list, }).catch((err) => { notifyError( - `Failed to reorder system modules: ${JSON.stringify( - err.response || err.message || err, - )}`, + `Failed to reorder system modules: ${describeError(err)}`, ); return err; }); @@ -596,11 +585,7 @@ export class SystemStateService extends AsyncHandler { ...this.active_item, modules: sorted_ids, }).catch((err) => { - notifyError( - `Failed to sort system modules: ${JSON.stringify( - err.response || err.message || err, - )}`, - ); + notifyError(`Failed to sort system modules: ${describeError(err)}`); return err; }); details.close(); @@ -624,9 +609,7 @@ export class SystemStateService extends AsyncHandler { zones: order, }).catch((err) => { notifyError( - `Failed to reorder system zones: ${JSON.stringify( - err.response || err.message || err, - )}`, + `Failed to reorder system zones: ${describeError(err)}`, ); return err; }); @@ -644,9 +627,9 @@ export class SystemStateService extends AsyncHandler { public async joinModule(id: string) { await addSystemModule(this.active_item.id, id).catch((err) => { notifyError( - `Error adding module ${id} to system. Error: ${ - err.statusText || err.message || err - }`, + `Error adding module ${id} to system. Error: ${describeError( + err, + )}`, ); }); this.timeout('join', async () => { @@ -674,9 +657,9 @@ export class SystemStateService extends AsyncHandler { device.id, ).catch((err) => { notifyError( - `Error removing module ${device.id} from system. Error: ${ - err.statusText || err.message || err - }`, + `Error removing module ${device.id} from system. Error: ${describeError( + err, + )}`, ); }); details.close(); @@ -698,9 +681,9 @@ export class SystemStateService extends AsyncHandler { zones, }).catch((err) => { notifyError( - `Error adding ${zone_list.length} zone(s) to system. Error: ${ - err.statusText || err.message || err - }`, + `Error adding ${zone_list.length} zone(s) to system. Error: ${describeError( + err, + )}`, ); }); if (system) this._state.replaceItem(system as unknown as Identity); @@ -724,9 +707,9 @@ export class SystemStateService extends AsyncHandler { zones, }).catch((err) => { notifyError( - `Error removing zone ${zone.id} from system. Error: ${ - err.statusText || err.message || err - }`, + `Error removing zone ${zone.id} from system. Error: ${describeError( + err, + )}`, ); }); details.close(); diff --git a/src/app/triggers/trigger-form.component.ts b/src/app/triggers/trigger-form.component.ts index 41f81db92..3b02c7f1e 100644 --- a/src/app/triggers/trigger-form.component.ts +++ b/src/app/triggers/trigger-form.component.ts @@ -21,6 +21,7 @@ import { updateTrigger, } from '@placeos/ts-client'; import { AsyncHandler } from '../common/async-handler.class'; +import { readError } from '../common/errors'; import { getInvalidSignalFields } from '../common/forms'; import { HotkeysService } from '../common/hotkeys.service'; import { i18n } from '../common/locale.service'; @@ -247,14 +248,12 @@ export class TriggerFormComponent extends AsyncHandler implements OnInit { settings_string, encryption_level: EncryptionLevel.Support, }); - await addSettings(new_settings).catch((err) => { + await addSettings(new_settings).catch(async (err) => { this.loading.set(null); notifyError( `Error saving settings for ${ item.name || item.id - }. Error: ${JSON.stringify( - err.response || err.message || err, - )}`, + }. Error: ${await readError(err)}`, ); }); } diff --git a/src/app/triggers/trigger-state.service.ts b/src/app/triggers/trigger-state.service.ts index b934a4dcc..1b2b9ecde 100644 --- a/src/app/triggers/trigger-state.service.ts +++ b/src/app/triggers/trigger-state.service.ts @@ -15,6 +15,7 @@ import { updateZone, } from '@placeos/ts-client'; +import { describeError } from '../common/errors'; import { escapeHtml } from '../common/general'; import { ActiveItemService } from '../common/item.service'; import { i18n } from '../common/locale.service'; @@ -178,12 +179,9 @@ export class TriggerStateService { }).catch((_) => _); details.close(); if (!(resp instanceof PlaceTrigger)) { - const error = resp as { response?: string; message?: string }; return notifyError( i18n('TRIGGERS.REORDER_CONFIRM_ERROR', { - error: JSON.stringify( - error.response || error.message || resp, - ), + error: describeError(resp), }), ); } @@ -230,12 +228,9 @@ export class TriggerStateService { }).catch((err) => err); details.close(); if (!(resp instanceof PlaceTrigger)) { - const error = resp as { response?: string; message?: string }; return notifyError( i18n('TRIGGERS.REMOVE_CONDITION_ERROR', { - error: JSON.stringify( - error.response || error.message || resp, - ), + error: describeError(resp), }), ); } @@ -282,12 +277,9 @@ export class TriggerStateService { }).catch((err) => err); details.close(); if (!(resp instanceof PlaceTrigger)) { - const error = resp as { response?: string; message?: string }; return notifyError( i18n('TRIGGERS.REMOVE_ACTION_ERROR', { - error: JSON.stringify( - error.response || error.message || resp, - ), + error: describeError(resp), }), ); } @@ -340,10 +332,7 @@ export class TriggerStateService { return notifyError( i18n('TRIGGERS.REMOVE_INSTANCE_ERROR', { type, - error: - (err as Record).responseText || - (err as Record).message || - err, + error: describeError(err), }), ); } diff --git a/src/app/ui/forms/settings-form.component.ts b/src/app/ui/forms/settings-form.component.ts index a59f3b137..19a2b86e2 100644 --- a/src/app/ui/forms/settings-form.component.ts +++ b/src/app/ui/forms/settings-form.component.ts @@ -30,6 +30,7 @@ import { MatProgressSpinnerModule } from '@angular/material/progress-spinner'; import { MatTabsModule } from '@angular/material/tabs'; import { MatTooltipModule } from '@angular/material/tooltip'; import * as yaml from 'js-yaml'; +import { readError } from '../../common/errors'; import { i18n } from '../../common/locale.service'; import { SettingsFieldComponent } from '../custom-fields/settings-field.component'; import { IconComponent } from '../icon.component'; @@ -446,13 +447,11 @@ export class SettingsFormComponent extends AsyncHandler implements OnInit { ); this.clearChanges(); }, - (err) => { + async (err) => { this._setSaving(level, false); notifyError( i18n('COMMON.SETTINGS_SAVE_ERROR', { - error: JSON.stringify( - err.response || err.message || err, - ), + error: await readError(err), }), ); }, @@ -493,15 +492,13 @@ export class SettingsFormComponent extends AsyncHandler implements OnInit { notifySuccess(i18n('COMMON.SETTINGS_SAVE_SUCCESS_ALL')); this.clearChanges(); }, - (err) => { + async (err) => { for (const level of saved_levels) { this._setSaving(level, false); } notifyError( i18n('COMMON.SETTINGS_SAVE_ERROR', { - error: JSON.stringify( - err.response || err.message || err, - ), + error: await readError(err), }), ); }, diff --git a/src/app/ui/forms/trigger-action-modal.component.ts b/src/app/ui/forms/trigger-action-modal.component.ts index 8070d23e3..cfbd2d052 100644 --- a/src/app/ui/forms/trigger-action-modal.component.ts +++ b/src/app/ui/forms/trigger-action-modal.component.ts @@ -26,6 +26,7 @@ import { MatFormFieldModule } from '@angular/material/form-field'; import { MatInputModule } from '@angular/material/input'; import { MatSelectModule } from '@angular/material/select'; import { AsyncHandler } from '../../common/async-handler.class'; +import { describeError } from '../../common/errors'; import { i18n } from '../../common/locale.service'; import { notifyError, notifySuccess } from '../../common/notifications'; import { DialogEvent, Identity } from '../../common/types'; @@ -329,9 +330,7 @@ export class TriggerActionModalComponent notifyError( `Error ${ this.is_new ? 'adding' : 'updating' - } condition to trigger. Error: ${JSON.stringify( - err.response || err.message || err, - )}`, + } condition to trigger. Error: ${describeError(err)}`, ); }, ); diff --git a/src/app/ui/forms/trigger-condition-modal.component.ts b/src/app/ui/forms/trigger-condition-modal.component.ts index 07e7d66d2..faedb31b1 100644 --- a/src/app/ui/forms/trigger-condition-modal.component.ts +++ b/src/app/ui/forms/trigger-condition-modal.component.ts @@ -13,6 +13,7 @@ import { updateTrigger, } from '@placeos/ts-client'; import { AsyncHandler } from '../../common/async-handler.class'; +import { describeError } from '../../common/errors'; import { i18n } from '../../common/locale.service'; import { notifyError, notifySuccess } from '../../common/notifications'; import { DialogEvent, Identity } from '../../common/types'; @@ -156,7 +157,7 @@ export class TriggerConditionModalComponent extends AsyncHandler { }).catch((err) => notifyError( i18n('TRIGGERS.CONDITION_SAVE_ERROR', { - error: JSON.stringify(err.response || err.message || err), + error: describeError(err), }), ), ); diff --git a/src/app/ui/metadata-display.component.ts b/src/app/ui/metadata-display.component.ts index e93ca43b2..958e79b83 100644 --- a/src/app/ui/metadata-display.component.ts +++ b/src/app/ui/metadata-display.component.ts @@ -25,6 +25,7 @@ import { VERSION } from '../../env/version'; import { escapeHtml } from '../common/general'; // import { SchemaStateService } from '../admin/schema-state.service'; import { AsyncHandler } from '../common/async-handler.class'; +import { describeError } from '../common/errors'; import { notifyError, notifySuccess } from '../common/notifications'; import { HashMap } from '../common/types'; import { currentUser } from '../common/user-state'; @@ -338,9 +339,9 @@ export class MetadataDisplayComponent await removeMetadata(this.item().id, { name: field }).catch((err) => { result.close(); notifyError( - `Error removing old "${field}" metadata. Error: ${ - err.response || err.message || err - }`, + `Error removing old "${field}" metadata. Error: ${describeError( + err, + )}`, ); throw err; }); @@ -414,9 +415,7 @@ export class MetadataDisplayComponent notifyError( `Error removing old "${ field.name - }" metadata. Error: ${JSON.stringify( - err.response || err.message || err, - )}`, + }" metadata. Error: ${describeError(err)}`, ), ); } @@ -437,9 +436,7 @@ export class MetadataDisplayComponent notifyError( `Error saving "${ value.name - }" metadata. Error: ${JSON.stringify( - err.response || err.message || err, - )}`, + }" metadata. Error: ${describeError(err)}`, ); }); } @@ -520,9 +517,7 @@ export class MetadataDisplayComponent }) .catch((err) => notifyError( - `Error loading metadata. Error: ${ - err.response || err.message || err - }`, + `Error loading metadata. Error: ${describeError(err)}`, ), ) .finally(() => this.loading_list.set(false)); diff --git a/src/app/zones/zone-form.component.ts b/src/app/zones/zone-form.component.ts index 339ddab99..9ee0c3ce1 100644 --- a/src/app/zones/zone-form.component.ts +++ b/src/app/zones/zone-form.component.ts @@ -29,6 +29,7 @@ import { MAT_DIALOG_DATA, MatDialogRef } from '@angular/material/dialog'; import { MatFormFieldModule } from '@angular/material/form-field'; import { MatInputModule } from '@angular/material/input'; import { AsyncHandler } from '../common/async-handler.class'; +import { readError } from '../common/errors'; import { addSignalChipItem, getInvalidSignalFields, @@ -476,14 +477,12 @@ export class ZoneFormComponent extends AsyncHandler implements OnInit { settings_string, encryption_level: EncryptionLevel.Support, }); - await addSettings(new_settings).catch((err) => { + await addSettings(new_settings).catch(async (err) => { this.loading.set(null); notifyError( `Error saving settings for ${ item.name || item.id - }. Error: ${JSON.stringify( - err.response || err.message || err, - )}`, + }. Error: ${await readError(err)}`, ); }); } diff --git a/src/app/zones/zones-state.service.ts b/src/app/zones/zones-state.service.ts index edc95c96d..12e6d3ad8 100644 --- a/src/app/zones/zones-state.service.ts +++ b/src/app/zones/zones-state.service.ts @@ -18,6 +18,7 @@ import { updateGroupZone, updateZone, } from '@placeos/ts-client'; +import { describeError } from '../common/errors'; import { escapeHtml, unique } from '../common/general'; import { ActiveItemService } from '../common/item.service'; import { i18n } from '../common/locale.service'; @@ -263,9 +264,9 @@ export class ZonesStateService { }).catch((err) => { details.close(); notifyError( - `Error removing trigger ${trigger.id} from zone. Error: ${ - err.statusText || err.message || err - }`, + `Error removing trigger ${trigger.id} from zone. Error: ${describeError( + err, + )}`, ); throw err; }); diff --git a/src/tests/common/errors.spec.ts b/src/tests/common/errors.spec.ts new file mode 100644 index 000000000..248428d01 --- /dev/null +++ b/src/tests/common/errors.spec.ts @@ -0,0 +1,58 @@ +import { describe, expect, it } from 'vitest'; +import { describeError, readError } from '../../app/common/errors'; + +describe('describeError', () => { + it('turns a ts-client Response rejection into its status', () => { + const error = new Response('', { + status: 403, + statusText: 'Forbidden', + }); + expect(describeError(error)).toBe('403 Forbidden'); + }); + + it('uses an Error message when there is one', () => { + expect(describeError(new Error('gateway'))).toBe('gateway'); + }); + + it('falls back to a status shape without a message', () => { + expect(describeError({ status: 502, statusText: 'Bad Gateway' })).toBe( + '502 Bad Gateway', + ); + }); + + it('never renders an empty reason or "{}"', () => { + expect(describeError(undefined)).toBe('Unknown error'); + expect(describeError({})).toBe('Unknown error'); + }); +}); + +describe('readError', () => { + it('adds the message from a JSON body', async () => { + const error = new Response('{"message":"name taken"}', { + status: 422, + statusText: 'Unprocessable Entity', + }); + expect(await readError(error)).toBe( + '422 Unprocessable Entity: name taken', + ); + }); + + it('adds a plain text body and leaves the original readable', async () => { + const error = new Response('driver failed to compile', { + status: 500, + statusText: 'Internal Server Error', + }); + expect(await readError(error)).toBe( + '500 Internal Server Error: driver failed to compile', + ); + expect(await error.text()).toBe('driver failed to compile'); + }); + + it('skips an HTML body and a body that is already read', async () => { + const html = new Response('oops', { status: 502 }); + expect(await readError(html)).toBe('502 request failed'); + const used = new Response('gone', { status: 404, statusText: 'Nope' }); + await used.text(); + expect(await readError(used)).toBe('404 Nope'); + }); +}); diff --git a/src/tests/overlays/confirm-modal.component.spec.ts b/src/tests/overlays/confirm-modal.component.spec.ts index 14a18c84f..c924af610 100644 --- a/src/tests/overlays/confirm-modal.component.spec.ts +++ b/src/tests/overlays/confirm-modal.component.spec.ts @@ -38,7 +38,6 @@ import { CONFIRM_METADATA, ConfirmModalComponent, ConfirmModalData, - describeError, openConfirmModal, receiptToTsv, } from '../../app/overlays/confirm-modal.component'; @@ -771,32 +770,3 @@ describe('CONFIRM_METADATA', () => { expect(CONFIRM_METADATA.height).toBe('auto'); }); }); - -describe('describeError', () => { - it('turns a ts-client Response rejection into something readable', () => { - // ts-client throws the raw Response for any non-OK status, and - // interpolating that gives "[object Response]" — which is what the - // user would otherwise be told is the reason they cannot continue. - expect(describeError(new Response('', { status: 403 }))).toContain( - '403', - ); - expect(describeError(new Response('', { status: 403 }))).not.toContain( - 'object Response', - ); - }); - - it('uses an Error message when there is one', () => { - expect(describeError(new Error('gateway'))).toBe('gateway'); - }); - - it('falls back to a status shape without a message', () => { - expect(describeError({ status: 502, statusText: 'Bad Gateway' })).toBe( - '502 Bad Gateway', - ); - }); - - it('never renders an empty reason', () => { - expect(describeError(undefined)).toBe('Unknown error'); - expect(describeError({})).toBe('Unknown error'); - }); -}); From 5e2d982f2beb2abf472410da5179b22e722ced05 Mon Sep 17 00:00:00 2001 From: Alex Sorafumo Date: Wed, 30 Sep 2026 11:03:42 +1000 Subject: [PATCH 2/9] fix: only show success toasts when the request succeeds Several actions caught the error, showed an error toast, then carried on to the success toast. Return early on failure instead: - recompileDriver - bulk driver update (now uses allSettled and reports the failed count) - joinModule, removeModule, addZones, removeZone on systems - system zones keep pending zones when the save fails Co-Authored-By: Claude Opus 5.5 (1M context) --- src/app/drivers/driver-state.service.ts | 28 ++++++++++-------- .../driver-update-list-modal.component.ts | 25 +++++++++++----- src/app/systems/system-state.service.ts | 29 ++++++++++++------- src/app/systems/system-zones.component.ts | 5 ++-- 4 files changed, 56 insertions(+), 31 deletions(-) diff --git a/src/app/drivers/driver-state.service.ts b/src/app/drivers/driver-state.service.ts index df4db2888..2612dc5f8 100644 --- a/src/app/drivers/driver-state.service.ts +++ b/src/app/drivers/driver-state.service.ts @@ -143,19 +143,23 @@ export class DriverStateService { ); if (details.reason !== 'done') return details.close(); details.loading('Recompiling driver... This may take a while.'); - await recompileDriver(item.id).catch(async (e) => { - console.log('Error:', e); - const content = e instanceof Response ? await e.text() : e; - this._last_error.set(content); - notifyError('Failed to recompile driver.', 'View Error', () => - this._dialog.open( - ViewResponseModalComponent, - { data: { content } }, - ), - ); - }); - notifySuccess('Successfully recompiled the driver.'); + const success = await recompileDriver(item.id) + .then(() => true) + .catch(async (e) => { + console.log('Error:', e); + const content = e instanceof Response ? await e.text() : e; + this._last_error.set(content); + notifyError('Failed to recompile driver.', 'View Error', () => + this._dialog.open( + ViewResponseModalComponent, + { data: { content } }, + ), + ); + return false; + }); details.close(); + if (!success) return; + notifySuccess('Successfully recompiled the driver.'); } public async reloadDriver() { diff --git a/src/app/drivers/driver-update-list-modal.component.ts b/src/app/drivers/driver-update-list-modal.component.ts index e2bfb4ac1..be7e392a6 100644 --- a/src/app/drivers/driver-update-list-modal.component.ts +++ b/src/app/drivers/driver-update-list-modal.component.ts @@ -7,6 +7,7 @@ import { MatDialogModule, MatDialogRef } from '@angular/material/dialog'; import { MatProgressSpinnerModule } from '@angular/material/progress-spinner'; import { MatTooltipModule } from '@angular/material/tooltip'; import { queryDrivers, updateDriver } from '@placeos/ts-client'; +import { describeError } from '../common/errors'; import { notifyError, notifySuccess } from '../common/notifications'; import { IconComponent } from '../ui/icon.component'; import { DateFromPipe } from '../ui/pipes/date-from.pipe'; @@ -225,7 +226,7 @@ export class DriverUpdateListModalComponent { const selected = drivers.data.filter((_) => this.selected_drivers().includes(_.id), ); - await Promise.all( + const results = await Promise.allSettled( selected.map((driver) => driver.commit !== driver.update_info.commit ? updateDriver(driver.id, { @@ -234,13 +235,23 @@ export class DriverUpdateListModalComponent { }) : Promise.resolve(), ), - ).catch((_) => { - notifyError('Error updating drivers', _); - this.loading.set(''); - this._dialog_ref.disableClose = false; - }); - notifySuccess(`Successfully updated ${selected.length} drivers`); + ); this.loading.set(''); + this._dialog_ref.disableClose = false; + const failed = results.filter( + (result): result is PromiseRejectedResult => + result.status === 'rejected', + ); + if (failed.length) { + notifyError( + `Failed to update ${failed.length} of ${selected.length} drivers. Error: ${describeError( + failed[0].reason, + )}`, + ); + this._change.set(Date.now()); + return; + } + notifySuccess(`Successfully updated ${selected.length} drivers`); if (this.all_selected()) this._dialog_ref.close(); else this._change.set(Date.now()); } diff --git a/src/app/systems/system-state.service.ts b/src/app/systems/system-state.service.ts index bbe2583d2..4c25cb7db 100644 --- a/src/app/systems/system-state.service.ts +++ b/src/app/systems/system-state.service.ts @@ -625,13 +625,17 @@ export class SystemStateService extends AsyncHandler { * @param id ID of the module to associate with the active system */ public async joinModule(id: string) { - await addSystemModule(this.active_item.id, id).catch((err) => { - notifyError( - `Error adding module ${id} to system. Error: ${describeError( - err, - )}`, - ); - }); + const added = await addSystemModule(this.active_item.id, id).catch( + (err) => { + notifyError( + `Error adding module ${id} to system. Error: ${describeError( + err, + )}`, + ); + return null; + }, + ); + if (!added) return; this.timeout('join', async () => { const system = await showSystem(this.active_item.id); if (system) this._state.replaceItem(system as unknown as Identity); @@ -663,13 +667,15 @@ export class SystemStateService extends AsyncHandler { ); }); details.close(); - if (system) this._state.replaceItem(system as unknown as Identity); + if (!system) return; + this._state.replaceItem(system as unknown as Identity); notifySuccess(`Successfully removed module from system.`); } /** * Add list of zones to the system * @param zones List of zones to add + * @returns Whether the zones were added */ public async addZones(zone_list: PlaceZone[]) { const zones = unique([ @@ -686,8 +692,10 @@ export class SystemStateService extends AsyncHandler { )}`, ); }); - if (system) this._state.replaceItem(system as unknown as Identity); + if (!system) return false; + this._state.replaceItem(system as unknown as Identity); notifySuccess(`Successfully added zone to system.`); + return true; } /** @@ -713,7 +721,8 @@ export class SystemStateService extends AsyncHandler { ); }); details.close(); - if (system) this._state.replaceItem(system as unknown as Identity); + if (!system) return; + this._state.replaceItem(system as unknown as Identity); notifySuccess(`Successfully removed zone from system.`); } diff --git a/src/app/systems/system-zones.component.ts b/src/app/systems/system-zones.component.ts index c12a793ea..626feddc1 100644 --- a/src/app/systems/system-zones.component.ts +++ b/src/app/systems/system-zones.component.ts @@ -266,8 +266,9 @@ export class SystemZonesComponent { public readonly savePendingZones = async () => { if (!this.pending_zones().length) return; - await this._service.addZones(this.pending_zones()); - this.pending_zones.set([]); + if (await this._service.addZones(this.pending_zones())) { + this.pending_zones.set([]); + } }; public readonly saveZoneOrder = async () => { From da142be2592470e7bad460d6ca67e49bf924957f Mon Sep 17 00:00:00 2001 From: Alex Sorafumo Date: Wed, 30 Sep 2026 11:08:15 +1000 Subject: [PATCH 3/9] fix: handle rejections that left spinners and modals stuck - Return after notifyError instead of rethrowing into template handlers (groups, users, zones, api keys, Azure integration) - Catch trigger selection errors and always close the select modal - Catch list loader errors and always clear the loading state (admin interfaces, edge, brokers, schemas, cluster, platform details, metadata history) - Validate schema JSON before save and catch save errors - Close the confirm modal and report the error on failed deletes (storage, uploads, api keys) - Read api key resources with hasValue(), as value() throws in the error state - Skip zones the user cannot load instead of failing the whole list - Use allSettled for auth sources so one failing type does not hide all - Report group and user membership load errors instead of showing an empty list - Authenticated images: check response.ok, ignore stale results, log failures, and bound the wait for the API authority - uploadFileWithPermissions always settles - Catch service worker update check failures Co-Authored-By: Claude Opus 5.5 (1M context) --- public/assets/locale/en-AU.json | 3 + src/app/admin/api-keys/api-keys.service.ts | 54 +++++---- src/app/admin/brokers.component.ts | 11 +- .../cluster-task-list.component.ts | 9 +- src/app/admin/details.component.ts | 29 +++-- src/app/admin/edge.component.ts | 17 ++- src/app/admin/interfaces.component.ts | 25 ++-- src/app/admin/schemas.component.ts | 39 ++++-- src/app/admin/storage/storage.component.ts | 6 + src/app/admin/upload-library.component.ts | 10 +- src/app/common/application.ts | 4 +- src/app/common/uploads.service.ts | 32 +++-- src/app/domains/domain-state.service.ts | 18 ++- src/app/groups/group-state.service.ts | 111 ++++++++++++------ .../metadata-history-modal.component.ts | 7 ++ src/app/systems/system-state.service.ts | 30 +++-- src/app/ui/authenticated-image.directive.ts | 21 +++- src/app/ui/upload-list.component.ts | 5 +- src/app/users/users-state.service.ts | 59 +++++++--- src/app/zones/zones-state.service.ts | 64 ++++++---- src/tests/groups/group-state.service.spec.ts | 14 ++- 21 files changed, 387 insertions(+), 181 deletions(-) diff --git a/public/assets/locale/en-AU.json b/public/assets/locale/en-AU.json index 61e2f9773..602f9c5a0 100644 --- a/public/assets/locale/en-AU.json +++ b/public/assets/locale/en-AU.json @@ -705,6 +705,7 @@ "GROUPS_BULK_ERROR": "Failed to add {{ count }} groups to user.", "GROUP_PERMISSIONS": "Group permissions", "GROUP_ADD_SUCCESS": "Successfully added user to group.", + "GROUPS_LOAD_ERROR": "Failed to load user groups. Error: {{ error }}", "GROUP_ADD_ERROR": "Failed to add user to group. Error: {{ error }}", "GROUP_SAVE_SUCCESS": "Successfully updated group permissions.", "GROUP_SAVE_ERROR": "Failed to update group permissions. Error: {{ error }}", @@ -791,6 +792,7 @@ "USERS_EMPTY": "No users associated with this group", "ZONES_EMPTY": "No zones associated with this group", "USER_ADD_SUCCESS": "Successfully added user to group.", + "USERS_LOAD_ERROR": "Failed to load group users. Error: {{ error }}", "USER_ADD_ERROR": "Failed to add user to group. Error: {{ error }}", "USER_SAVE_SUCCESS": "Successfully updated user permissions.", "USER_SAVE_ERROR": "Failed to update user permissions. Error: {{ error }}", @@ -800,6 +802,7 @@ "USER_REMOVE_SUCCESS": "Successfully removed user from group.", "USER_REMOVE_ERROR": "Failed to remove user from group. Error: {{ error }}", "ZONE_ADD_SUCCESS": "Successfully added zone to group.", + "ZONES_LOAD_ERROR": "Failed to load group zones. Error: {{ error }}", "ZONE_ADD_ERROR": "Failed to add zone to group. Error: {{ error }}", "ZONE_SAVE_SUCCESS": "Successfully updated zone permissions.", "ZONE_SAVE_ERROR": "Failed to update zone permissions. Error: {{ error }}", diff --git a/src/app/admin/api-keys/api-keys.service.ts b/src/app/admin/api-keys/api-keys.service.ts index 628ab27ba..458ffda72 100644 --- a/src/app/admin/api-keys/api-keys.service.ts +++ b/src/app/admin/api-keys/api-keys.service.ts @@ -12,6 +12,7 @@ import { update, } from '@placeos/ts-client'; import { addDays, getUnixTime } from 'date-fns'; +import { describeError } from '../../common/errors'; import { notifyError, notifySuccess } from '../../common/notifications'; import { waitForEvent } from '../../common/signals'; import { DialogEvent } from '../../common/types'; @@ -53,8 +54,8 @@ export class APIKeyService { loader: async () => (await get('/api/engine/v2/scopes')) as string[], }); - public readonly available_scopes = computed( - () => this._available_scopes.value() || [], + public readonly available_scopes = computed(() => + this._available_scopes.hasValue() ? this._available_scopes.value() : [], ); private readonly _available_keys = resource({ @@ -79,8 +80,8 @@ export class APIKeyService { }, }); - public readonly available_keys = computed( - () => this._available_keys.value() || [], + public readonly available_keys = computed(() => + this._available_keys.hasValue() ? this._available_keys.value() : [], ); private readonly _users = resource({ @@ -96,7 +97,9 @@ export class APIKeyService { }, }); - public readonly users = computed(() => this._users.value() || []); + public readonly users = computed(() => + this._users.hasValue() ? this._users.value() : [], + ); public setDomain(domain: PlaceDomain) { this._admin_data.setDomain('api-keys', domain); @@ -136,9 +139,10 @@ export class APIKeyService { }, }).catch((_) => { ref.close(); - notifyError(_); - throw _; + notifyError(describeError(_)); + return null; }); + if (!key) return; this._last_key.set(key as PlaceAPIKeyDetails); this._change.set(Date.now()); notifySuccess('Successfully created new API key.'); @@ -174,9 +178,10 @@ export class APIKeyService { expires_at: getUnixTime(addDays(Date.now(), 1)), // expire 1 day from creation }, }).catch((_) => { - notifyError(_); - throw _; + notifyError(describeError(_)); + return null; }); + if (!key) return; this._last_key.set(key as PlaceAPIKeyDetails); this._change.set(Date.now()); notifySuccess('Successfully created new API key.'); @@ -199,21 +204,22 @@ export class APIKeyService { if (details?.reason !== 'done') return; ref.componentInstance.loading.set('Updating API key...'); const domain = this._domain(); - await update({ - id: key.id, - query_params: {}, - fn: (d) => new PlaceAPIKeyDetails(d), - path: 'api_keys', - method: 'patch', - form_data: { - ...details.metadata, - authority_id: domain.id, - }, - }).catch((_) => { + try { + await update({ + id: key.id, + query_params: {}, + fn: (d) => new PlaceAPIKeyDetails(d), + path: 'api_keys', + method: 'patch', + form_data: { + ...details.metadata, + authority_id: domain.id, + }, + }); + } catch (_) { ref.close(); - notifyError(_); - throw _; - }); + return notifyError(describeError(_)); + } this._change.set(Date.now()); notifySuccess('Successfully updated API key.'); ref.close(); @@ -237,6 +243,8 @@ export class APIKeyService { query_params: {}, path: 'api_keys', }); + } catch (error) { + return notifyError(describeError(error)); } finally { details.close(); } diff --git a/src/app/admin/brokers.component.ts b/src/app/admin/brokers.component.ts index cb0092458..47c11580c 100644 --- a/src/app/admin/brokers.component.ts +++ b/src/app/admin/brokers.component.ts @@ -267,8 +267,13 @@ export class AdminBrokersComponent extends AsyncHandler implements OnInit { public async loadBrokers() { this.loading.set(true); - const brokers = await queryBrokers().then((r) => r.data); - this.brokers.set(brokers); - this.loading.set(false); + try { + const brokers = await queryBrokers().then((r) => r.data); + this.brokers.set(brokers); + } catch (err) { + notifyError(`Failed to load brokers. Error: ${describeError(err)}`); + } finally { + this.loading.set(false); + } } } diff --git a/src/app/admin/cluster-details/cluster-task-list.component.ts b/src/app/admin/cluster-details/cluster-task-list.component.ts index b689bbf71..1ca8898cf 100644 --- a/src/app/admin/cluster-details/cluster-task-list.component.ts +++ b/src/app/admin/cluster-details/cluster-task-list.component.ts @@ -293,7 +293,14 @@ export class PlaceClusterTaskListComponent public async loadCluster(id: string) { const clusters = await queryClusters({ q: id, - } as Record).then((_) => _.data); + } as Record) + .then((_) => _.data) + .catch((err) => { + notifyError( + `Failed to load cluster. Error: ${describeError(err)}`, + ); + return []; + }); // The search may return other clusters, so only accept an exact id const match = clusters.find((_) => _.id === id) || null; this.cluster.set(match); diff --git a/src/app/admin/details.component.ts b/src/app/admin/details.component.ts index 9b5bc3643..38f814f5f 100644 --- a/src/app/admin/details.component.ts +++ b/src/app/admin/details.component.ts @@ -262,24 +262,29 @@ export class PlaceDetailsComponent extends AsyncHandler implements OnInit { } public async loadPlatformDetails() { - const { changelog, version } = await get( - `${apiEndpoint()}/platform`, - ).catch((err) => { + void this.loadBackofficeChangelog(); + const platform = await get(`${apiEndpoint()}/platform`).catch((err) => { notifyError( i18n('ADMIN.BACKEND_SERVICES_ERROR', { error: describeError(err), }), ); - throw err; + return null; }); - this.changelog_data.set(changelog.replace('# Changelog\n\n', '')); - this.backend_version.set(version); - this.backoffice_logs.set( - await ( - await fetch( - 'https://raw.githubusercontent.com/PlaceOS/backoffice/develop/CHANGELOG.md', - ) - ).text(), + if (!platform) return; + const { changelog, version } = platform; + this.changelog_data.set( + (changelog ?? '').replace('# Changelog\n\n', ''), ); + this.backend_version.set(version); + } + + /** Loads the backoffice changelog from GitHub. Failure only hides the link. */ + private async loadBackofficeChangelog() { + const response = await fetch( + 'https://raw.githubusercontent.com/PlaceOS/backoffice/develop/CHANGELOG.md', + ).catch(() => null); + if (!response?.ok) return; + this.backoffice_logs.set(await response.text().catch(() => '')); } } diff --git a/src/app/admin/edge.component.ts b/src/app/admin/edge.component.ts index 043d91a75..85793dcc4 100644 --- a/src/app/admin/edge.component.ts +++ b/src/app/admin/edge.component.ts @@ -254,10 +254,17 @@ export class PlaceEdgeComponent implements OnInit { public async loadEdges() { this.loading.set('Loading edge node list...'); - const { data } = await queryEdges(); - this.edge_list.set( - (data || []).sort((a, b) => a.id?.localeCompare(b.id)), - ); - this.loading.set(''); + try { + const { data } = await queryEdges(); + this.edge_list.set( + (data || []).sort((a, b) => a.id?.localeCompare(b.id)), + ); + } catch (err) { + notifyError( + `Failed to load edge nodes. Error: ${describeError(err)}`, + ); + } finally { + this.loading.set(''); + } } } diff --git a/src/app/admin/interfaces.component.ts b/src/app/admin/interfaces.component.ts index a84ece7e2..c6aaf071b 100644 --- a/src/app/admin/interfaces.component.ts +++ b/src/app/admin/interfaces.component.ts @@ -2,6 +2,8 @@ import { Component, OnInit, signal } from '@angular/core'; import { listInterfaceRepositories } from '@placeos/ts-client'; import { MatProgressBarModule } from '@angular/material/progress-bar'; +import { describeError } from '../common/errors'; +import { notifyError } from '../common/notifications'; import { SimpleTableComponent } from '../ui/simple-table.component'; import { TranslatePipe } from '../ui/translate.pipe'; @@ -68,13 +70,20 @@ export class AdminInterfacesComponent implements OnInit { public async loadInterfaces() { this.loading.set(true); - const mapping = await listInterfaceRepositories(); - const list = Object.keys(mapping).map((id) => ({ - id, - name: mapping[id], - })); - list.sort((a, b) => `${a.id}`?.localeCompare(`${b.id}`)); - this.interfaces.set(list); - this.loading.set(false); + try { + const mapping = await listInterfaceRepositories(); + const list = Object.keys(mapping).map((id) => ({ + id, + name: mapping[id], + })); + list.sort((a, b) => `${a.id}`?.localeCompare(`${b.id}`)); + this.interfaces.set(list); + } catch (err) { + notifyError( + `Failed to load interfaces. Error: ${describeError(err)}`, + ); + } finally { + this.loading.set(false); + } } } diff --git a/src/app/admin/schemas.component.ts b/src/app/admin/schemas.component.ts index 1051f26ff..1635595c5 100644 --- a/src/app/admin/schemas.component.ts +++ b/src/app/admin/schemas.component.ts @@ -6,6 +6,8 @@ import { MatInputModule } from '@angular/material/input'; import { MatSelectModule } from '@angular/material/select'; import { MatTooltipModule } from '@angular/material/tooltip'; import { create, query, update } from '@placeos/ts-client'; +import { describeError } from '../common/errors'; +import { notifyError } from '../common/notifications'; import { SettingsFieldComponent } from '../ui/custom-fields/settings-field.component'; import { IconComponent } from '../ui/icon.component'; import { TranslatePipe } from '../ui/translate.pipe'; @@ -151,6 +153,13 @@ export class AdminSchemasComponent implements OnInit { public async saveSchema() { const schema = this.schema_copy(); + try { + JSON.parse(schema.schema || '{}'); + } catch (err) { + return notifyError( + `Schema is not valid JSON. ${describeError(err)}`, + ); + } let schema_list = this.schema_list(); const details = { query_params: {}, @@ -158,13 +167,20 @@ export class AdminSchemasComponent implements OnInit { form_data: schema, path: 'schema', }; - const new_schema = await (schema.id - ? update({ - ...details, - id: schema.id, - method: 'patch', - }) - : create({ ...details })); + let new_schema: JsonSchema; + try { + new_schema = await (schema.id + ? update({ + ...details, + id: schema.id, + method: 'patch', + }) + : create({ ...details })); + } catch (err) { + return notifyError( + `Failed to save schema. Error: ${describeError(err)}`, + ); + } schema_list = [ ...schema_list.filter((_) => schema.id !== _.id), new_schema, @@ -196,7 +212,14 @@ export class AdminSchemasComponent implements OnInit { ..._, }) as JsonSchema, path: 'schema', - }).then((_) => _.data as JsonSchema[]); + }) + .then((_) => _.data as JsonSchema[]) + .catch((err) => { + notifyError( + `Failed to load schemas. Error: ${describeError(err)}`, + ); + return [] as JsonSchema[]; + }); schema_list.sort((a, b) => a.name?.localeCompare(b.name)); this.schema_list.set(schema_list); } diff --git a/src/app/admin/storage/storage.component.ts b/src/app/admin/storage/storage.component.ts index 3ddcdb22f..a383613be 100644 --- a/src/app/admin/storage/storage.component.ts +++ b/src/app/admin/storage/storage.component.ts @@ -7,7 +7,9 @@ import { MatProgressBarModule } from '@angular/material/progress-bar'; import { MatSelectModule } from '@angular/material/select'; import { MatTooltipModule } from '@angular/material/tooltip'; import { escapeHtml } from '../../common/general'; +import { describeError } from '../../common/errors'; import { i18n } from '../../common/locale.service'; +import { notifyError } from '../../common/notifications'; import { openConfirmModal } from '../../overlays/confirm-modal.component'; import { IconComponent } from '../../ui/icon.component'; import { DateFromPipe } from '../../ui/pipes/date-from.pipe'; @@ -232,6 +234,10 @@ export class StorageComponent implements OnInit { resp.loading(i18n('ADMIN.STORAGE_REMOVE_LOADING')); try { await removeStorage(item.id); + } catch (err) { + return notifyError( + `Failed to remove storage provider. Error: ${describeError(err)}`, + ); } finally { resp.close(); } diff --git a/src/app/admin/upload-library.component.ts b/src/app/admin/upload-library.component.ts index 7fa109f1d..b2fd089d2 100644 --- a/src/app/admin/upload-library.component.ts +++ b/src/app/admin/upload-library.component.ts @@ -20,6 +20,7 @@ import { MatTooltipModule } from '@angular/material/tooltip'; import { apiKey, cleanObject, query, remove, token } from '@placeos/ts-client'; import { AsyncHandler } from '../common/async-handler.class'; import { escapeHtml } from '../common/general'; +import { describeError } from '../common/errors'; import { i18n } from '../common/locale.service'; import { notifyError, @@ -464,7 +465,10 @@ export class UploadLibraryComponent extends AsyncHandler implements OnInit { const uploads = []; for (let i = 0; i < files.length; i++) { uploads.push( - this._uploads.uploadFileWithPermissions(files[i]), + // A cancelled upload needs no feedback + this._uploads + .uploadFileWithPermissions(files[i]) + .catch(() => null), ); } // Cancelling the permissions modal rejects that file only @@ -566,6 +570,10 @@ export class UploadLibraryComponent extends AsyncHandler implements OnInit { query_params: {}, path: 'uploads', }); + } catch (err) { + return notifyError( + `Failed to remove upload. Error: ${describeError(err)}`, + ); } finally { result.close(); } diff --git a/src/app/common/application.ts b/src/app/common/application.ts index 54255f46c..a8b882b88 100644 --- a/src/app/common/application.ts +++ b/src/app/common/application.ts @@ -23,7 +23,9 @@ export function setupCache(cache: SwUpdate, interval: number = 5 * 60 * 1000) { if (_timer) clearInterval(_timer); _timer = setInterval(() => { log('CACHE', `Checking for updates...`); - checkForUpdate(cache); + checkForUpdate(cache).catch((error) => + log('CACHE', 'Update check failed', [error], 'warn', true), + ); }, interval); } } diff --git a/src/app/common/uploads.service.ts b/src/app/common/uploads.service.ts index 446639257..286cc943f 100644 --- a/src/app/common/uploads.service.ts +++ b/src/app/common/uploads.service.ts @@ -26,28 +26,26 @@ export class UploadsService { this._upload_list.set(in_progress_list); } + /** + * Asks the user for upload permissions, then uploads the file. + * Rejects when the user cancels the permissions modal. + */ public async uploadFileWithPermissions(file: File) { const { UploadPermissionsModalComponent } = await import( '../ui/upload-permissions-modal.component' ); - return new Promise((resolve, reject) => { - const ref = this._dialog.open(UploadPermissionsModalComponent, { - data: { file }, - }); - lastValueFrom(ref.afterClosed()).then(async (details) => { - if (details) { - const id = await this.uploadFile( - details.file, - details.is_public, - details.permissions, - ).catch((e) => { - reject(e); - throw e; - }); - resolve(id); - } else reject(); - }); + const ref = this._dialog.open(UploadPermissionsModalComponent, { + data: { file }, }); + const details = await lastValueFrom(ref.afterClosed(), { + defaultValue: null, + }); + if (!details) throw new Error('Upload cancelled'); + return this.uploadFile( + details.file, + details.is_public, + details.permissions, + ); } public uploadFile( diff --git a/src/app/domains/domain-state.service.ts b/src/app/domains/domain-state.service.ts index b91940a44..5bf3636d1 100644 --- a/src/app/domains/domain-state.service.ts +++ b/src/app/domains/domain-state.service.ts @@ -143,11 +143,15 @@ export class DomainStateService { authority_id: params.id, ...(params.full ? {} : { limit: 1 }), }; - const responses = await Promise.all([ + // One failing source type should not hide the others + const results = await Promise.allSettled([ querySAMLSources(q), queryOAuthSources(q), queryLDAPSources(q), - ]).catch(() => []); + ]); + const responses = results + .filter((result) => result.status === 'fulfilled') + .map((result) => result.value); return { data: responses.flatMap( (response): PlaceAuthSource[] => response.data, @@ -215,10 +219,14 @@ export class DomainStateService { const result = await get( `/api/engine/v2/admin_consent/${encodeURIComponent(item.id)}`, ).catch((error) => { - notifyError(i18n('DOMAINS.AZURE_INTEGRATION_ERROR', { error })); - throw error; + notifyError( + i18n('DOMAINS.AZURE_INTEGRATION_ERROR', { + error: describeError(error), + }), + ); + return null; }); - if (result.url) { + if (result?.url) { window.open(result.url, '_blank', 'noopener noreferrer'); } } diff --git a/src/app/groups/group-state.service.ts b/src/app/groups/group-state.service.ts index 6b28ca0e5..b32faf907 100644 --- a/src/app/groups/group-state.service.ts +++ b/src/app/groups/group-state.service.ts @@ -18,6 +18,7 @@ import { updateGroupZone, } from '@placeos/ts-client'; import { escapeHtml } from '../common/general'; +import { describeError } from '../common/errors'; import { ActiveItemService } from '../common/item.service'; import { i18n } from '../common/locale.service'; import { notifyError, notifySuccess } from '../common/notifications'; @@ -48,7 +49,14 @@ export class GroupStateService { const response = await queryGroupUsers({ group_id: item.id, limit: 1000, - }).catch(() => ({ data: [] })); + }).catch((error) => { + notifyError( + i18n('GROUPS.USERS_LOAD_ERROR', { + error: describeError(error), + }), + ); + return { data: [] as PlaceGroupUser[] }; + }); return response.data.sort((a, b) => (a.user?.name || a.user_id).localeCompare( b.user?.name || b.user_id, @@ -72,7 +80,14 @@ export class GroupStateService { const response = await queryGroupZones({ group_id: item.id, limit: 1000, - }).catch(() => ({ data: [] })); + }).catch((error) => { + notifyError( + i18n('GROUPS.ZONES_LOAD_ERROR', { + error: describeError(error), + }), + ); + return { data: [] as PlaceGroupZone[] }; + }); return response.data.sort((a, b) => (a.zone?.name || a.zone_id).localeCompare( b.zone?.name || b.zone_id, @@ -113,13 +128,16 @@ export class GroupStateService { public async addUser(user: PlaceUser) { if (!user?.id) return; - await addGroupUser({ - group_id: this.active_item.id, - user_id: user.id, - }).catch((error) => { - notifyError(i18n('GROUPS.USER_ADD_ERROR', { error })); - throw error; - }); + try { + await addGroupUser({ + group_id: this.active_item.id, + user_id: user.id, + }); + } catch (error) { + return notifyError( + i18n('GROUPS.USER_ADD_ERROR', { error: describeError(error) }), + ); + } notifySuccess(i18n('GROUPS.USER_ADD_SUCCESS')); this.changed(); } @@ -259,23 +277,31 @@ export class GroupStateService { ); if (details.reason !== 'done') return; details.loading(i18n('GROUPS.USER_REMOVE_LOADING')); - await removeGroupUser(item.user_id, item.group_id).catch((error) => { + try { + await removeGroupUser(item.user_id, item.group_id); + } catch (error) { details.close(); - notifyError(i18n('GROUPS.USER_REMOVE_ERROR', { error })); - throw error; - }); + return notifyError( + i18n('GROUPS.USER_REMOVE_ERROR', { + error: describeError(error), + }), + ); + } details.close(); notifySuccess(i18n('GROUPS.USER_REMOVE_SUCCESS')); this.changed(); } public async updateUser(item: PlaceGroupUser) { - await updateGroupUser(item.user_id, item.group_id, { - permissions: +item.permissions || 0, - }).catch((error) => { - notifyError(i18n('GROUPS.USER_SAVE_ERROR', { error })); - throw error; - }); + try { + await updateGroupUser(item.user_id, item.group_id, { + permissions: +item.permissions || 0, + }); + } catch (error) { + return notifyError( + i18n('GROUPS.USER_SAVE_ERROR', { error: describeError(error) }), + ); + } notifySuccess(i18n('GROUPS.USER_SAVE_SUCCESS')); this.changed(); } @@ -297,13 +323,16 @@ export class GroupStateService { public async addZone(zone: PlaceZone) { if (!zone?.id) return; - await addGroupZone({ - group_id: this.active_item.id, - zone_id: zone.id, - }).catch((error) => { - notifyError(i18n('GROUPS.ZONE_ADD_ERROR', { error })); - throw error; - }); + try { + await addGroupZone({ + group_id: this.active_item.id, + zone_id: zone.id, + }); + } catch (error) { + return notifyError( + i18n('GROUPS.ZONE_ADD_ERROR', { error: describeError(error) }), + ); + } notifySuccess(i18n('GROUPS.ZONE_ADD_SUCCESS')); this.changed(); } @@ -321,24 +350,32 @@ export class GroupStateService { ); if (details.reason !== 'done') return; details.loading(i18n('GROUPS.ZONE_REMOVE_LOADING')); - await removeGroupZone(item.group_id, item.zone_id).catch((error) => { + try { + await removeGroupZone(item.group_id, item.zone_id); + } catch (error) { details.close(); - notifyError(i18n('GROUPS.ZONE_REMOVE_ERROR', { error })); - throw error; - }); + return notifyError( + i18n('GROUPS.ZONE_REMOVE_ERROR', { + error: describeError(error), + }), + ); + } details.close(); notifySuccess(i18n('GROUPS.ZONE_REMOVE_SUCCESS')); this.changed(); } public async updateZone(item: PlaceGroupZone) { - await updateGroupZone(item.group_id, item.zone_id, { - permissions: +item.permissions || 0, - deny: !!item.deny, - }).catch((error) => { - notifyError(i18n('GROUPS.ZONE_SAVE_ERROR', { error })); - throw error; - }); + try { + await updateGroupZone(item.group_id, item.zone_id, { + permissions: +item.permissions || 0, + deny: !!item.deny, + }); + } catch (error) { + return notifyError( + i18n('GROUPS.ZONE_SAVE_ERROR', { error: describeError(error) }), + ); + } notifySuccess(i18n('GROUPS.ZONE_SAVE_SUCCESS')); this.changed(); } diff --git a/src/app/overlays/metadata-history-modal.component.ts b/src/app/overlays/metadata-history-modal.component.ts index a7f86e48b..2a2256aaf 100644 --- a/src/app/overlays/metadata-history-modal.component.ts +++ b/src/app/overlays/metadata-history-modal.component.ts @@ -12,6 +12,8 @@ import { FormsModule } from '@angular/forms'; import { MatFormFieldModule } from '@angular/material/form-field'; import { MatSelectModule } from '@angular/material/select'; import { listMetadataHistory, PlaceMetadata } from '@placeos/ts-client'; +import { describeError } from '../common/errors'; +import { notifyError } from '../common/notifications'; import { DiffViewerComponent } from '../ui/diff-viewer.component'; import { IconComponent } from '../ui/icon.component'; import { TranslatePipe } from '../ui/translate.pipe'; @@ -189,6 +191,11 @@ export class MetadataHistoryModalComponent implements OnInit { const history = await listMetadataHistory(this._data.id, { name: this._data.name, limit: 5000, + }).catch((err) => { + notifyError( + `Failed to load metadata history. Error: ${describeError(err)}`, + ); + return [] as PlaceMetadata[]; }); this.history.set(history); } diff --git a/src/app/systems/system-state.service.ts b/src/app/systems/system-state.service.ts index 4c25cb7db..291dbd18d 100644 --- a/src/app/systems/system-state.service.ts +++ b/src/app/systems/system-state.service.ts @@ -220,9 +220,14 @@ export class SystemStateService extends AsyncHandler { try { const response = isSubsystemUser() ? { - data: await Promise.all( - item.zones.map((id) => showZone(id)), - ), + // A zone the user cannot see must not hide the rest + data: ( + await Promise.all( + item.zones.map((id) => + showZone(id).catch(() => null), + ), + ) + ).filter((zone) => !!zone), } : await listSystemZones(item.id).catch(() => ({ data: [], @@ -418,12 +423,19 @@ export class SystemStateService extends AsyncHandler { waitForEvent(ref.afterClosed()), ]); if (details?.reason !== 'action') return ref.close(); - const t = await this.addTrigger( - ref.componentInstance.item as PlaceTrigger, - ); - ref.close(); - this.changed(); - return t; + try { + const t = await this.addTrigger( + ref.componentInstance.item as PlaceTrigger, + ); + this.changed(); + return t; + } catch (err) { + notifyError( + `Error adding trigger to system. Error: ${describeError(err)}`, + ); + } finally { + ref.close(); + } } public async addTrigger(trigger: PlaceTrigger) { diff --git a/src/app/ui/authenticated-image.directive.ts b/src/app/ui/authenticated-image.directive.ts index 2678a73bd..f9872787e 100644 --- a/src/app/ui/authenticated-image.directive.ts +++ b/src/app/ui/authenticated-image.directive.ts @@ -10,6 +10,8 @@ import { apiKey, authority, token } from '@placeos/ts-client'; import { AsyncHandler } from '../common/async-handler.class'; const IMAGE_STORE = new Map(); +/** Tries to wait for the API authority before giving up (300ms apart) */ +const MAX_AUTH_WAIT_ATTEMPTS = 100; @Directive({ selector: 'img [auth],video [auth]', @@ -23,12 +25,19 @@ export class AuthenticatedImageDirective public readonly source = input(undefined); public ngOnChanges(changes: SimpleChanges) { - if (changes.source && this.source()) this._loadImage().catch(); + if (changes.source && this.source()) this._load(); } - private async _loadImage() { + private _load(attempt = 0) { + this._loadImage(attempt).catch((e) => + console.warn('Failed to load image:', e), + ); + } + + private async _loadImage(attempt: number) { if (!this._image_el || !authority()) { - return this.timeout('load', () => this._loadImage().catch(), 300); + if (attempt >= MAX_AUTH_WAIT_ATTEMPTS) return; + return this.timeout('load', () => this._load(attempt + 1), 300); } // If not an API call, just load the image const source = this.source(); @@ -50,9 +59,15 @@ export class AuthenticatedImageDirective location.protocol === 'https:' ? 'secure;' : '' }`; const response = await fetch(source); + // Do not cache an error body, such as a 401, as the image + if (!response.ok) { + throw new Error(`${response.status} ${response.statusText}`); + } const blob = await response.blob(); const url = URL.createObjectURL(blob); IMAGE_STORE.set(source, url); + // The source changed while this one loaded + if (this.source() !== source) return; this._image_el.nativeElement.src = url; } } diff --git a/src/app/ui/upload-list.component.ts b/src/app/ui/upload-list.component.ts index 2f40eaca2..cf31aa750 100644 --- a/src/app/ui/upload-list.component.ts +++ b/src/app/ui/upload-list.component.ts @@ -287,7 +287,10 @@ export class UploadListComponent extends AsyncHandler implements OnInit { if (files.length) { this.show.set(true); for (let i = 0; i < files.length; i++) { - this._uploads.uploadFileWithPermissions(files[i]); + // A cancelled upload needs no feedback + this._uploads + .uploadFileWithPermissions(files[i]) + .catch(() => null); } } } diff --git a/src/app/users/users-state.service.ts b/src/app/users/users-state.service.ts index aa7f2d27a..5b18fecb3 100644 --- a/src/app/users/users-state.service.ts +++ b/src/app/users/users-state.service.ts @@ -13,6 +13,7 @@ import { updateGroupUser, } from '@placeos/ts-client'; import { escapeHtml } from '../common/general'; +import { describeError } from '../common/errors'; import { ActiveItemService } from '../common/item.service'; import { i18n } from '../common/locale.service'; import { notifyError, notifySuccess } from '../common/notifications'; @@ -91,7 +92,14 @@ export class UsersStateService { const response = await queryGroupUsers({ user_id: item.id, limit: 1000, - }).catch(() => ({ data: [] })); + }).catch((error) => { + notifyError( + i18n('USERS.GROUPS_LOAD_ERROR', { + error: describeError(error), + }), + ); + return { data: [] as PlaceGroupUser[] }; + }); return response.data.sort((a, b) => (a.group?.name || a.group_id).localeCompare( b.group?.name || b.group_id, @@ -111,13 +119,16 @@ export class UsersStateService { public async addGroup(group: PlaceGroup) { if (!group?.id) return; - await addGroupUser({ - user_id: this.active_item.id, - group_id: group.id, - }).catch((error) => { - notifyError(i18n('USERS.GROUP_ADD_ERROR', { error })); - throw error; - }); + try { + await addGroupUser({ + user_id: this.active_item.id, + group_id: group.id, + }); + } catch (error) { + return notifyError( + i18n('USERS.GROUP_ADD_ERROR', { error: describeError(error) }), + ); + } notifySuccess(i18n('USERS.GROUP_ADD_SUCCESS')); this.changed(); } @@ -199,23 +210,31 @@ export class UsersStateService { ); if (details.reason !== 'done') return; details.loading(i18n('USERS.GROUP_REMOVE_LOADING')); - await removeGroupUser(item.user_id, item.group_id).catch((error) => { + try { + await removeGroupUser(item.user_id, item.group_id); + } catch (error) { details.close(); - notifyError(i18n('USERS.GROUP_REMOVE_ERROR', { error })); - throw error; - }); + return notifyError( + i18n('USERS.GROUP_REMOVE_ERROR', { + error: describeError(error), + }), + ); + } details.close(); notifySuccess(i18n('USERS.GROUP_REMOVE_SUCCESS')); this.changed(); } public async updateGroup(item: PlaceGroupUser) { - await updateGroupUser(item.user_id, item.group_id, { - permissions: +item.permissions || 0, - }).catch((error) => { - notifyError(i18n('USERS.GROUP_SAVE_ERROR', { error })); - throw error; - }); + try { + await updateGroupUser(item.user_id, item.group_id, { + permissions: +item.permissions || 0, + }); + } catch (error) { + return notifyError( + i18n('USERS.GROUP_SAVE_ERROR', { error: describeError(error) }), + ); + } notifySuccess(i18n('USERS.GROUP_SAVE_SUCCESS')); this.changed(); } @@ -262,7 +281,9 @@ export class UsersStateService { this.changed(); notifySuccess(i18n('USERS.REVIVE_SUCCESS')); } catch (error) { - notifyError(i18n('USERS.REVIVE_ERROR', { error })); + notifyError( + i18n('USERS.REVIVE_ERROR', { error: describeError(error) }), + ); } finally { details.close(); } diff --git a/src/app/zones/zones-state.service.ts b/src/app/zones/zones-state.service.ts index 12e6d3ad8..2a4224d16 100644 --- a/src/app/zones/zones-state.service.ts +++ b/src/app/zones/zones-state.service.ts @@ -228,11 +228,18 @@ export class ZonesStateService { waitForEvent(ref.afterClosed()), ]); if (details?.reason !== 'action') return ref.close(); - const zone = await this.addTrigger( - ref.componentInstance.item as PlaceTrigger, - ); - ref.close(); - if (zone) this._service.replaceItem(zone as unknown as Identity); + try { + const zone = await this.addTrigger( + ref.componentInstance.item as PlaceTrigger, + ); + if (zone) this._service.replaceItem(zone as unknown as Identity); + } catch (err) { + notifyError( + `Error adding trigger to zone. Error: ${describeError(err)}`, + ); + } finally { + ref.close(); + } } public async addTrigger( @@ -277,13 +284,16 @@ export class ZonesStateService { public async addGroup(group: PlaceGroup) { if (!group?.id) return; - await addGroupZone({ - group_id: group.id, - zone_id: this.active_item.id, - }).catch((error) => { - notifyError(i18n('ZONES.GROUP_ADD_ERROR', { error })); - throw error; - }); + try { + await addGroupZone({ + group_id: group.id, + zone_id: this.active_item.id, + }); + } catch (error) { + return notifyError( + i18n('ZONES.GROUP_ADD_ERROR', { error: describeError(error) }), + ); + } notifySuccess(i18n('ZONES.GROUP_ADD_SUCCESS')); this.changed(); } @@ -357,24 +367,32 @@ export class ZonesStateService { ); if (details.reason !== 'done') return; details.loading(i18n('ZONES.GROUP_REMOVE_LOADING')); - await removeGroupZone(item.group_id, item.zone_id).catch((error) => { + try { + await removeGroupZone(item.group_id, item.zone_id); + } catch (error) { details.close(); - notifyError(i18n('ZONES.GROUP_REMOVE_ERROR', { error })); - throw error; - }); + return notifyError( + i18n('ZONES.GROUP_REMOVE_ERROR', { + error: describeError(error), + }), + ); + } details.close(); notifySuccess(i18n('ZONES.GROUP_REMOVE_SUCCESS')); this.changed(); } public async updateGroup(item: PlaceGroupZone) { - await updateGroupZone(item.group_id, item.zone_id, { - permissions: +item.permissions || 0, - deny: !!item.deny, - }).catch((error) => { - notifyError(i18n('ZONES.GROUP_SAVE_ERROR', { error })); - throw error; - }); + try { + await updateGroupZone(item.group_id, item.zone_id, { + permissions: +item.permissions || 0, + deny: !!item.deny, + }); + } catch (error) { + return notifyError( + i18n('ZONES.GROUP_SAVE_ERROR', { error: describeError(error) }), + ); + } notifySuccess(i18n('ZONES.GROUP_SAVE_SUCCESS')); this.changed(); } diff --git a/src/tests/groups/group-state.service.spec.ts b/src/tests/groups/group-state.service.spec.ts index bb3b41520..e727ac446 100644 --- a/src/tests/groups/group-state.service.spec.ts +++ b/src/tests/groups/group-state.service.spec.ts @@ -129,7 +129,7 @@ describe('group membership actions', () => { }); }); - it('recovers to empty lists after membership queries fail', async () => { + it('reports failed membership queries and recovers to empty lists', async () => { mocks.queryGroupUsers.mockRejectedValue(new Error('Offline')); mocks.queryGroupZones.mockRejectedValue(new Error('Offline')); active.set(group); @@ -138,6 +138,12 @@ describe('group membership actions', () => { expect(service.zones()).toEqual([]); expect(service.counts()).toEqual({ users: 0, zones: 0 }); expect(service.loading()).toBe(false); + expect(mocks.notifyError).toHaveBeenCalledWith( + 'GROUPS.USERS_LOAD_ERROR', + ); + expect(mocks.notifyError).toHaveBeenCalledWith( + 'GROUPS.ZONES_LOAD_ERROR', + ); }); it('does not send an add request for an unsaved user', async () => { @@ -148,9 +154,7 @@ describe('group membership actions', () => { it('reports a failed add without claiming success', async () => { const error = new Error('Forbidden'); mocks.addGroupUser.mockRejectedValue(error); - await expect( - service.addUser(new PlaceUser({ id: 'user-1' })), - ).rejects.toBe(error); + await service.addUser(new PlaceUser({ id: 'user-1' })); expect(mocks.addGroupUser).toHaveBeenCalledExactlyOnceWith({ group_id: 'group-1', user_id: 'user-1', @@ -192,7 +196,7 @@ describe('group membership actions', () => { it('closes the confirmation and reports a failed zone removal', async () => { const error = new Error('Forbidden'); mocks.removeGroupZone.mockRejectedValue(error); - await expect(service.removeZone(zone)).rejects.toBe(error); + await service.removeZone(zone); expect(mocks.removeGroupZone).toHaveBeenCalledExactlyOnceWith( 'group-1', 'zone-1', From eea968a709bbd26051487257f28ae4103224e7f4 Mon Sep 17 00:00:00 2001 From: Alex Sorafumo Date: Wed, 30 Sep 2026 11:09:50 +1000 Subject: [PATCH 4/9] fix: derive loading state from each resource Several loaders toggled one shared `_loading` signal, so the first loader to finish hid the progress bar while others still ran. Loaders that returned a promise without awaiting it cleared the flag at once. Derive `loading` from the resources' isLoading() in the group, user, zone, driver and trigger state services. Bulk adds keep their own flag. The system module loader now ignores results from an aborted load or a system that is no longer active, so it cannot overwrite the current module list or clear its loading flag. Co-Authored-By: Claude Opus 5.5 (1M context) --- src/app/drivers/driver-state.service.ts | 25 ++--- src/app/groups/group-state.service.ts | 88 +++++++++--------- src/app/systems/system-state.service.ts | 8 +- src/app/triggers/trigger-state.service.ts | 10 +- src/app/users/users-state.service.ts | 51 +++++----- src/app/zones/zones-state.service.ts | 98 ++++++++++---------- src/tests/groups/group-state.service.spec.ts | 2 + 7 files changed, 132 insertions(+), 150 deletions(-) diff --git a/src/app/drivers/driver-state.service.ts b/src/app/drivers/driver-state.service.ts index 2612dc5f8..62d7c46d2 100644 --- a/src/app/drivers/driver-state.service.ts +++ b/src/app/drivers/driver-state.service.ts @@ -24,7 +24,6 @@ export class DriverStateService { private _state = inject(ActiveItemService); private _dialog = inject(MatDialog); - private _loading = signal(false); private _last_error = signal(null); private _poll = signal(0); private _modules_change = signal(0); @@ -33,7 +32,9 @@ export class DriverStateService { () => this._state.item() as unknown as PlaceDriver, ); - public readonly loading = this._loading.asReadonly(); + public readonly loading = computed( + () => this._modules.isLoading() || this._docs.isLoading(), + ); /** Bumped each time a module of the driver is removed */ public readonly modules_change = this._modules_change.asReadonly(); @@ -56,15 +57,10 @@ export class DriverStateService { params: () => ({ item: this.item(), changed: this._modules_change() }), loader: async ({ params: { item } }) => { if (!(item instanceof PlaceDriver)) return [] as PlaceModule[]; - this._loading.set(true); - try { - const response = await queryModules({ - driver_id: item.id, - }).catch(() => ({ data: [] })); - return response.data; - } finally { - this._loading.set(false); - } + const response = await queryModules({ + driver_id: item.id, + }).catch(() => ({ data: [] })); + return response.data; }, }); @@ -74,12 +70,7 @@ export class DriverStateService { params: () => this.item(), loader: async ({ params: item }) => { if (!(item instanceof PlaceDriver)) return ''; - this._loading.set(true); - try { - return driverReadme(item.id).catch(() => ''); - } finally { - this._loading.set(false); - } + return driverReadme(item.id).catch(() => ''); }, }); diff --git a/src/app/groups/group-state.service.ts b/src/app/groups/group-state.service.ts index b32faf907..3e277b18f 100644 --- a/src/app/groups/group-state.service.ts +++ b/src/app/groups/group-state.service.ts @@ -32,39 +32,40 @@ export class GroupStateService { private _state = inject(ActiveItemService); private _dialog = inject(MatDialog); private _changed = signal(0); - private _loading = signal(false); + /** Set while a bulk add runs */ + private _saving = signal(false); public readonly item = computed( () => this._state.item() as unknown as PlaceGroup, ); - public readonly loading = this._loading.asReadonly(); + public readonly loading = computed( + () => + this._saving() || + this._users.isLoading() || + this._zones.isLoading(), + ); private readonly _users = resource({ params: () => ({ item: this.item(), changed: this._changed() }), loader: async ({ params }) => { const { item } = params; if (!(item instanceof PlaceGroup)) return [] as PlaceGroupUser[]; - this._loading.set(true); - try { - const response = await queryGroupUsers({ - group_id: item.id, - limit: 1000, - }).catch((error) => { - notifyError( - i18n('GROUPS.USERS_LOAD_ERROR', { - error: describeError(error), - }), - ); - return { data: [] as PlaceGroupUser[] }; - }); - return response.data.sort((a, b) => - (a.user?.name || a.user_id).localeCompare( - b.user?.name || b.user_id, - ), + const response = await queryGroupUsers({ + group_id: item.id, + limit: 1000, + }).catch((error) => { + notifyError( + i18n('GROUPS.USERS_LOAD_ERROR', { + error: describeError(error), + }), ); - } finally { - this._loading.set(false); - } + return { data: [] as PlaceGroupUser[] }; + }); + return response.data.sort((a, b) => + (a.user?.name || a.user_id).localeCompare( + b.user?.name || b.user_id, + ), + ); }, }); @@ -75,27 +76,22 @@ export class GroupStateService { loader: async ({ params }) => { const { item } = params; if (!(item instanceof PlaceGroup)) return [] as PlaceGroupZone[]; - this._loading.set(true); - try { - const response = await queryGroupZones({ - group_id: item.id, - limit: 1000, - }).catch((error) => { - notifyError( - i18n('GROUPS.ZONES_LOAD_ERROR', { - error: describeError(error), - }), - ); - return { data: [] as PlaceGroupZone[] }; - }); - return response.data.sort((a, b) => - (a.zone?.name || a.zone_id).localeCompare( - b.zone?.name || b.zone_id, - ), + const response = await queryGroupZones({ + group_id: item.id, + limit: 1000, + }).catch((error) => { + notifyError( + i18n('GROUPS.ZONES_LOAD_ERROR', { + error: describeError(error), + }), ); - } finally { - this._loading.set(false); - } + return { data: [] as PlaceGroupZone[] }; + }); + return response.data.sort((a, b) => + (a.zone?.name || a.zone_id).localeCompare( + b.zone?.name || b.zone_id, + ), + ); }, }); @@ -180,7 +176,7 @@ export class GroupStateService { const users = result?.items; if (!users?.length) return; const permissions = +result.permissions || 0; - this._loading.set(true); + this._saving.set(true); const results = await Promise.allSettled( users.map((user) => addGroupUser({ @@ -190,7 +186,7 @@ export class GroupStateService { }), ), ); - this._loading.set(false); + this._saving.set(false); const failed = results.filter((_) => _.status === 'rejected').length; if (failed) { notifyError(i18n('GROUPS.USERS_BULK_ERROR', { count: failed })); @@ -243,7 +239,7 @@ export class GroupStateService { ); const zones = result?.items; if (!zones?.length) return; - this._loading.set(true); + this._saving.set(true); const results = await Promise.allSettled( zones.map((zone) => addGroupZone({ @@ -252,7 +248,7 @@ export class GroupStateService { }), ), ); - this._loading.set(false); + this._saving.set(false); const failed = results.filter((_) => _.status === 'rejected').length; if (failed) { notifyError(i18n('GROUPS.ZONES_BULK_ERROR', { count: failed })); diff --git a/src/app/systems/system-state.service.ts b/src/app/systems/system-state.service.ts index 291dbd18d..7c1bf1c5a 100644 --- a/src/app/systems/system-state.service.ts +++ b/src/app/systems/system-state.service.ts @@ -135,7 +135,7 @@ export class SystemStateService extends AsyncHandler { private readonly _module_resource = resource({ params: () => ({ item: this.item(), changed: this._change() }), - loader: async ({ params }) => { + loader: async ({ params, abortSignal }) => { const { item } = params; if (!(item instanceof PlaceSystem)) { this._last_module_system = ''; @@ -152,6 +152,10 @@ export class SystemStateService extends AsyncHandler { complete: true, limit: 200, } as Record).catch(() => ({ data: [] })); + // A newer load or another system took over. Leave its state alone. + if (abortSignal.aborted || this.item()?.id !== item.id) { + return [] as PlaceModule[]; + } // Keep known connection state across refreshes. Bindings only // emit on change, so a reset value would never be filled again. const known_state = new Map( @@ -182,7 +186,7 @@ export class SystemStateService extends AsyncHandler { this._modules.set(modules); return modules; } finally { - this.setLoading('modules', false); + if (!abortSignal.aborted) this.setLoading('modules', false); } }, }); diff --git a/src/app/triggers/trigger-state.service.ts b/src/app/triggers/trigger-state.service.ts index 1b2b9ecde..8f303d6a5 100644 --- a/src/app/triggers/trigger-state.service.ts +++ b/src/app/triggers/trigger-state.service.ts @@ -38,24 +38,18 @@ export class TriggerStateService { private _dialog = inject(MatDialog); private _change = signal(0); - private _loading = signal(false); public readonly item = computed( () => this._service.item() as unknown as PlaceTrigger, ); - public readonly loading = this._loading.asReadonly(); + public readonly loading = computed(() => this._instances.isLoading()); private readonly _instances = resource({ params: () => ({ item: this.item(), changed: this._change() }), loader: async ({ params }) => { const { item } = params; if (!(item instanceof PlaceTrigger)) return [] as PlaceTrigger[]; - this._loading.set(true); - try { - return listTriggerInstances(item.id).catch(() => []); - } finally { - this._loading.set(false); - } + return listTriggerInstances(item.id).catch(() => []); }, }); diff --git a/src/app/users/users-state.service.ts b/src/app/users/users-state.service.ts index 5b18fecb3..7b17ef819 100644 --- a/src/app/users/users-state.service.ts +++ b/src/app/users/users-state.service.ts @@ -31,10 +31,16 @@ export class UsersStateService { private _service = inject(ActiveItemService); private _dialog = inject(MatDialog); - private _loading = signal(false); + /** Set while a bulk add runs */ + private _saving = signal(false); private _change = signal(0); - public readonly loading = this._loading.asReadonly(); + public readonly loading = computed( + () => + this._saving() || + this._counts.isLoading() || + this._groups.isLoading(), + ); public readonly item = this._service.item; @@ -46,7 +52,6 @@ export class UsersStateService { loader: async ({ params }) => { const { item } = params; if (!(item instanceof PlaceUser)) return {}; - this._loading.set(true); const details = await Promise.all([ listMetadata(item.id) .then((d) => d.length) @@ -56,7 +61,6 @@ export class UsersStateService { .catch(() => 0), ]); const [metadata, groups] = details; - this._loading.set(false); return { metadata, groups, @@ -87,27 +91,22 @@ export class UsersStateService { loader: async ({ params }) => { const { item } = params; if (!(item instanceof PlaceUser)) return [] as PlaceGroupUser[]; - this._loading.set(true); - try { - const response = await queryGroupUsers({ - user_id: item.id, - limit: 1000, - }).catch((error) => { - notifyError( - i18n('USERS.GROUPS_LOAD_ERROR', { - error: describeError(error), - }), - ); - return { data: [] as PlaceGroupUser[] }; - }); - return response.data.sort((a, b) => - (a.group?.name || a.group_id).localeCompare( - b.group?.name || b.group_id, - ), + const response = await queryGroupUsers({ + user_id: item.id, + limit: 1000, + }).catch((error) => { + notifyError( + i18n('USERS.GROUPS_LOAD_ERROR', { + error: describeError(error), + }), ); - } finally { - this._loading.set(false); - } + return { data: [] as PlaceGroupUser[] }; + }); + return response.data.sort((a, b) => + (a.group?.name || a.group_id).localeCompare( + b.group?.name || b.group_id, + ), + ); }, }); @@ -175,7 +174,7 @@ export class UsersStateService { if (!groups?.length) return; // Only send permissions when set so the backend default applies const permissions = +result.permissions || 0; - this._loading.set(true); + this._saving.set(true); const results = await Promise.allSettled( groups.map((group) => addGroupUser({ @@ -185,7 +184,7 @@ export class UsersStateService { }), ), ); - this._loading.set(false); + this._saving.set(false); const failed = results.filter((_) => _.status === 'rejected').length; if (failed) { notifyError(i18n('USERS.GROUPS_BULK_ERROR', { count: failed })); diff --git a/src/app/zones/zones-state.service.ts b/src/app/zones/zones-state.service.ts index 2a4224d16..f5153589c 100644 --- a/src/app/zones/zones-state.service.ts +++ b/src/app/zones/zones-state.service.ts @@ -44,10 +44,16 @@ export class ZonesStateService { private _service = inject(ActiveItemService); private _dialog = inject(MatDialog); - private _loading = signal(false); + /** Set while a bulk add runs */ + private _saving = signal(false); private _change = signal(0); - public readonly loading = this._loading.asReadonly(); + public readonly loading = computed( + () => + this._saving() || + this._counts.isLoading() || + this._groups.isLoading(), + ); public readonly item = computed( () => this._service.item() as unknown as PlaceZone, @@ -58,39 +64,34 @@ export class ZonesStateService { loader: async ({ params }) => { const { item } = params; if (!(item instanceof PlaceZone)) return {}; - this._loading.set(true); - try { - const details = await Promise.all([ - querySystems({ zone_id: item.id, limit: 1 }) - .then((d) => d.total) - .catch(() => 0), - (isSubsystemUser() - ? Promise.resolve({ data: [], total: 0 }) - : listZoneTriggers(item.id) - ) - .then((d) => d.total) - .catch(() => 0), - listMetadata(item.id) - .then((d) => d.length) - .catch(() => 0), - queryZones({ parent_id: item.id, limit: 1 }) - .then((d) => d.total) - .catch(() => 0), - queryGroupZones({ zone_id: item.id, limit: 1 }) - .then((d) => d.total) - .catch(() => 0), - ]); - const [systems, triggers, metadata, children, groups] = details; - return { - systems, - triggers, - metadata, - children, - groups, - }; - } finally { - this._loading.set(false); - } + const details = await Promise.all([ + querySystems({ zone_id: item.id, limit: 1 }) + .then((d) => d.total) + .catch(() => 0), + (isSubsystemUser() + ? Promise.resolve({ data: [], total: 0 }) + : listZoneTriggers(item.id) + ) + .then((d) => d.total) + .catch(() => 0), + listMetadata(item.id) + .then((d) => d.length) + .catch(() => 0), + queryZones({ parent_id: item.id, limit: 1 }) + .then((d) => d.total) + .catch(() => 0), + queryGroupZones({ zone_id: item.id, limit: 1 }) + .then((d) => d.total) + .catch(() => 0), + ]); + const [systems, triggers, metadata, children, groups] = details; + return { + systems, + triggers, + metadata, + children, + groups, + }; }, }); @@ -173,20 +174,15 @@ export class ZonesStateService { loader: async ({ params }) => { const { item } = params; if (!(item instanceof PlaceZone)) return [] as PlaceGroupZone[]; - this._loading.set(true); - try { - const response = await queryGroupZones({ - zone_id: item.id, - limit: 1000, - }).catch(() => ({ data: [] })); - return response.data.sort((a, b) => - (a.group?.name || a.group_id).localeCompare( - b.group?.name || b.group_id, - ), - ); - } finally { - this._loading.set(false); - } + const response = await queryGroupZones({ + zone_id: item.id, + limit: 1000, + }).catch(() => ({ data: [] })); + return response.data.sort((a, b) => + (a.group?.name || a.group_id).localeCompare( + b.group?.name || b.group_id, + ), + ); }, }); @@ -333,7 +329,7 @@ export class ZonesStateService { .afterClosed(), ); if (!groups?.length) return; - this._loading.set(true); + this._saving.set(true); const results = await Promise.allSettled( groups.map((group) => addGroupZone({ @@ -342,7 +338,7 @@ export class ZonesStateService { }), ), ); - this._loading.set(false); + this._saving.set(false); const failed = results.filter((_) => _.status === 'rejected').length; if (failed) { notifyError(i18n('ZONES.GROUPS_BULK_ERROR', { count: failed })); diff --git a/src/tests/groups/group-state.service.spec.ts b/src/tests/groups/group-state.service.spec.ts index e727ac446..6f6318557 100644 --- a/src/tests/groups/group-state.service.spec.ts +++ b/src/tests/groups/group-state.service.spec.ts @@ -266,6 +266,8 @@ describe('group membership actions', () => { expect(mocks.notifySuccess).toHaveBeenCalledExactlyOnceWith( 'GROUPS.USERS_BULK_SUCCESS:1', ); + // The lists reload after the bulk add + await TestBed.inject(ApplicationRef).whenStable(); expect(service.loading()).toBe(false); }); From 03e218d27802085a6a16c3d298af30237730b879 Mon Sep 17 00:00:00 2001 From: Alex Sorafumo Date: Wed, 30 Sep 2026 11:13:12 +1000 Subject: [PATCH 5/9] fix: bound polling and fix timer handles - AsyncHandler.timeout clears the stored handle before it calls the callback, so a callback that reschedules itself under the same name keeps its new handle. The name and callback error messages were swapped. - waitForSignalValue rejects after a max wait (60s by default). Guards, settings, item service and app init handle the timeout. - App init uses the bounded wait in place of its own 30s user timer, and catches staff tenant check failures. - Image list uploads start the status poll only after an upload starts, and a cancelled permissions modal no longer leaks the interval. - Remove the second current user loader in user-state.ts. It could leave current_user null forever. BackofficeUsersService now publishes the user there, so current_user and BackofficeUsersService.user are the same signal. Co-Authored-By: Claude Opus 5.5 (1M context) --- src/app/app.ts | 21 ++++-- src/app/common/async-handler.class.ts | 12 ++-- src/app/common/item.service.ts | 15 ++++- src/app/common/settings.service.ts | 6 +- src/app/common/signals.ts | 17 +++-- src/app/common/user-state.ts | 28 +++----- .../image-list-field.component.ts | 24 ++++--- src/app/ui/global-loading.component.ts | 5 +- src/app/ui/guards/authorised-admin.guard.ts | 4 +- src/app/ui/guards/authorised-user.guard.ts | 4 +- src/app/users/users.service.ts | 10 +-- src/tests/common/async-handler.class.spec.ts | 31 +++++++-- src/tests/common/signals.spec.ts | 14 ++++ src/tests/common/user-state.spec.ts | 66 ++++--------------- 14 files changed, 146 insertions(+), 111 deletions(-) diff --git a/src/app/app.ts b/src/app/app.ts index 2f153525f..de0a09783 100644 --- a/src/app/app.ts +++ b/src/app/app.ts @@ -203,7 +203,11 @@ export class AppComponent extends AsyncHandler implements OnInit { this.loading.set(true); setLoadingMessage('Loading application settings...'); /** Wait for settings to initialise */ - await waitForSignalValue(this._settings.initialised, (_) => _); + const settings_ready = await waitForSignalValue( + this._settings.initialised, + (_) => _, + ).catch(() => false); + if (!settings_ready) return this.onInitError(); const settings = (this._settings.get('composer') || {}) as PlaceSettings; settings.mock = !!this._settings.get('mock'); @@ -212,9 +216,13 @@ export class AppComponent extends AsyncHandler implements OnInit { /** Wait for authentication details to load */ await setupPlace(settings).catch(() => this.onInitError()); setupCache(this._cache); - this.timeout('wait_for_user', () => this.onInitError(), 30 * 1000); - await waitForSignalValue(this._users.initialised, (_) => _); - this.clearTimeout('wait_for_user'); + const user_ready = await waitForSignalValue( + this._users.initialised, + (_) => _, + 50, + 30 * 1000, + ).catch(() => false); + if (!user_ready) return this.onInitError(); setLoadingMessage('Initialising locales...'); // TranslatePipe is pure, so load translations before the shell renders await this._initLocale(); @@ -236,7 +244,10 @@ export class AppComponent extends AsyncHandler implements OnInit { } }); setLoadingMessage('Checking staff tenants...'); - this._checkTenants(); + // Runs after the user loads, so currentUser() is set here + this._checkTenants().catch((error) => + log('Init', 'Failed to check staff tenants', [error], 'warn'), + ); } private onInitError() { diff --git a/src/app/common/async-handler.class.ts b/src/app/common/async-handler.class.ts index b1dc5400a..7fa25a5e2 100644 --- a/src/app/common/async-handler.class.ts +++ b/src/app/common/async-handler.class.ts @@ -48,14 +48,16 @@ export class AsyncHandler implements OnDestroy { if (name && fn && fn instanceof Function) { this.clearTimeout(name); this._timers[name] = setTimeout(() => { - fn(); + // Clear first, so a callback that schedules itself again + // under the same name keeps its new handle this._timers[name] = null; + fn(); }, delay); } else { throw new Error( name - ? 'Cannot create named timeout without a name' - : 'Cannot create a timeout without a callback', + ? 'Cannot create a timeout without a callback' + : 'Cannot create named timeout without a name', ); } } @@ -84,8 +86,8 @@ export class AsyncHandler implements OnDestroy { } else { throw new Error( name - ? 'Cannot create named interval without a name' - : 'Cannot create a interval without a callback', + ? 'Cannot create an interval without a callback' + : 'Cannot create named interval without a name', ); } } diff --git a/src/app/common/item.service.ts b/src/app/common/item.service.ts index 43366422c..46602babc 100644 --- a/src/app/common/item.service.ts +++ b/src/app/common/item.service.ts @@ -201,7 +201,11 @@ export class ActiveItemService extends AsyncHandler { /** Update the active item */ public async setItem(id: string) { const request = ++this._item_request; - await waitForSignalValue(this._user.user, (user) => !!user); + const user = await waitForSignalValue( + this._user.user, + (user) => !!user, + ).catch(() => null); + if (!user) return; if (!hasSupportRole() && !hasSupportSubsystem()) return; const scope_version = this._scope_version; if ( @@ -625,7 +629,14 @@ export class ActiveItemService extends AsyncHandler { 'update', async () => { if (!this.actions) return; - await waitForSignalValue(this._user.user, (user) => !!user); + const user = await waitForSignalValue( + this._user.user, + (user) => !!user, + ).catch(() => null); + if (!user) { + this._loading_list.set(false); + return; + } if ( list_version !== this._list_version || type !== this._type || diff --git a/src/app/common/settings.service.ts b/src/app/common/settings.service.ts index 0907ebaf0..15406f258 100644 --- a/src/app/common/settings.service.ts +++ b/src/app/common/settings.service.ts @@ -108,7 +108,11 @@ export class SettingsService extends AsyncHandler { if (!window.application) window.application = {}; window.application.settings = this; } - const user = await waitForSignalValue(current_user, (user) => !!user); + const user = await waitForSignalValue( + current_user, + (user) => !!user, + ).catch(() => null); + if (!user) return; const data = await showMetadata(user.id, 'settings'); this._user_settings.set((data.details || {}) as HashMap); this._initDarkMode(); diff --git a/src/app/common/signals.ts b/src/app/common/signals.ts index 34d58f147..c3bc93c18 100644 --- a/src/app/common/signals.ts +++ b/src/app/common/signals.ts @@ -74,19 +74,26 @@ export function waitForEvent( }); } +/** + * Resolves with the first value of the signal that passes the predicate. + * Checks every `delay` ms, and rejects after `max_wait` ms so a value that + * never arrives cannot leave the caller waiting forever. + */ export function waitForSignalValue( source: Signal, predicate: (value: T) => boolean = () => true, delay = 50, + max_wait = 60 * 1000, ): Promise { - return new Promise((resolve) => { + return new Promise((resolve, reject) => { + const started = Date.now(); const check = () => { const value = source(); - if (predicate(value)) { - resolve(value); - } else { - setTimeout(check, delay); + if (predicate(value)) return resolve(value); + if (Date.now() - started >= max_wait) { + return reject(new Error('Timed out waiting for signal value')); } + setTimeout(check, delay); }; check(); }); diff --git a/src/app/common/user-state.ts b/src/app/common/user-state.ts index 8e6870f75..56dd351fa 100644 --- a/src/app/common/user-state.ts +++ b/src/app/common/user-state.ts @@ -1,29 +1,21 @@ import { signal } from '@angular/core'; -import { PlaceUser, showUser } from '@placeos/ts-client'; +import { PlaceUser } from '@placeos/ts-client'; const EMPTY_USER = new PlaceUser(); +/** + * The signed in user. `BackofficeUsersService` loads it and is the only + * writer. It lives here so that code which cannot inject that service, such + * as `SettingsService` (a dependency of it), can still read the user. + */ const _current_user = signal(null); export const current_user = _current_user.asReadonly(); -declare let jest; - -setTimeout(async () => { - try { - if (jest) return; - } catch { - // jest not defined, continue - } - for (let i = 0; i < 10; i++) { - await new Promise((resolve) => setTimeout(resolve, 1000)); - const user = await showUser('current').catch(() => null); - if (user) { - _current_user.set(user); - return; - } - } -}, 300); +/** Publish the signed in user. Only `BackofficeUsersService` calls this. */ +export function setCurrentUser(user: PlaceUser) { + _current_user.set(user); +} /** Get the current user details */ export function currentUser() { diff --git a/src/app/ui/custom-fields/image-list-field.component.ts b/src/app/ui/custom-fields/image-list-field.component.ts index 2a7197461..2c01b3b27 100644 --- a/src/app/ui/custom-fields/image-list-field.component.ts +++ b/src/app/ui/custom-fields/image-list-field.component.ts @@ -324,14 +324,20 @@ export class ImageListFieldComponent const files: FileList = element.files; /* istanbul ignore else */ if (files.length) { - this.interval('update_status', () => - this._updateUploadHistory(), - ); - for (let i = 0; i < files.length; i++) { - const id = await this._uploads.uploadFileWithPermissions( - files[i], - ); - this.upload_ids.update((list) => [...list, id]); + try { + for (let i = 0; i < files.length; i++) { + const id = + await this._uploads.uploadFileWithPermissions( + files[i], + ); + this.upload_ids.update((list) => [...list, id]); + // Start polling only once there is an upload to track + this.interval('update_status', () => + this._updateUploadHistory(), + ); + } + } catch { + // The user cancelled the upload permissions modal } } } @@ -363,7 +369,7 @@ export class ImageListFieldComponent private async _updateUploadHistory() { const list = this.upload_ids(); - if (list.length === 0) return; + if (list.length === 0) return this.clearInterval('update_status'); const global_list = this._uploads.upload_list(); const new_list = global_list.filter((_) => list.find((i) => i === _.id), diff --git a/src/app/ui/global-loading.component.ts b/src/app/ui/global-loading.component.ts index 63d71595b..0fa2b5582 100644 --- a/src/app/ui/global-loading.component.ts +++ b/src/app/ui/global-loading.component.ts @@ -57,7 +57,10 @@ export class GlobalLoadingComponent extends AsyncHandler implements OnInit { public async ngOnInit() { this.loading.set(true); - await waitForSignalValue(this._settings.initialised, (_) => _); + // Keep checking for a token even if settings are slow to load + await waitForSignalValue(this._settings.initialised, (_) => _).catch( + () => null, + ); this.online.set(isOnline()); this.interval( 'has_token', diff --git a/src/app/ui/guards/authorised-admin.guard.ts b/src/app/ui/guards/authorised-admin.guard.ts index 967af8d24..1d208247a 100644 --- a/src/app/ui/guards/authorised-admin.guard.ts +++ b/src/app/ui/guards/authorised-admin.guard.ts @@ -29,7 +29,7 @@ export class AuthorisedAdminGuard { const user: PlaceUser = await waitForSignalValue( this._users.user, (_) => !!_, - ); + ).catch(() => null); const can_activate = !!user && (user.sys_admin || @@ -49,7 +49,7 @@ export class AuthorisedAdminGuard { const user: PlaceUser = await waitForSignalValue( this._users.user, (_) => !!_, - ); + ).catch(() => null); const can_activate = !!user && (user.sys_admin || diff --git a/src/app/ui/guards/authorised-user.guard.ts b/src/app/ui/guards/authorised-user.guard.ts index 284fecdf3..513dca3f6 100644 --- a/src/app/ui/guards/authorised-user.guard.ts +++ b/src/app/ui/guards/authorised-user.guard.ts @@ -29,7 +29,7 @@ export class AuthorisedUserGuard { const user: PlaceUser = await waitForSignalValue( this._users.user, (_) => !!_, - ); + ).catch(() => null); const can_activate = !!user && (user.sys_admin || @@ -49,7 +49,7 @@ export class AuthorisedUserGuard { const user: PlaceUser = await waitForSignalValue( this._users.user, (_) => !!_, - ); + ).catch(() => null); const can_activate = !!user && (user.sys_admin || diff --git a/src/app/users/users.service.ts b/src/app/users/users.service.ts index cb0f80cd3..0d41ba58b 100644 --- a/src/app/users/users.service.ts +++ b/src/app/users/users.service.ts @@ -13,6 +13,7 @@ import { AsyncHandler } from '../common/async-handler.class'; import { SettingsService } from '../common/settings.service'; import { loadSupportAccess } from '../common/support-access'; import { FilterFn } from '../common/types'; +import { current_user, setCurrentUser } from '../common/user-state'; import * as Sentry from '@sentry/browser'; import { addDays } from 'date-fns'; @@ -28,11 +29,10 @@ export class BackofficeUsersService extends AsyncHandler { /** Signal with the currently available list of users */ public readonly listing = signal([]); - private _user = signal(null); /** Active User */ - public readonly user = this._user.asReadonly(); + public readonly user = current_user; /** Active User */ - public readonly current = () => this._user(); + public readonly current = () => this.user(); /** Active User */ public readonly currentSignal = () => this.user; @@ -49,7 +49,7 @@ export class BackofficeUsersService extends AsyncHandler { : false; const theme = localStorage.getItem('BACKOFFICE.theme') ?? - ((this._user() || {}) as Record).ui_theme; + ((this.user() || {}) as Record).ui_theme; return (theme && theme === 'dark') || (!theme && os_dark); } public set dark_mode(state: boolean) { @@ -101,7 +101,7 @@ export class BackofficeUsersService extends AsyncHandler { return; } await loadSupportAccess(user); - this._user.set(user); + setCurrentUser(user); Sentry.withScope((scope) => scope.setUser({ email: user.email }), ); diff --git a/src/tests/common/async-handler.class.spec.ts b/src/tests/common/async-handler.class.spec.ts index 8401e04d1..fdfa96c13 100644 --- a/src/tests/common/async-handler.class.spec.ts +++ b/src/tests/common/async-handler.class.spec.ts @@ -123,11 +123,15 @@ describe('AsyncHandler', () => { }); it('should throw error without name', () => { - expect(() => handler.testTimeout('', vi.fn())).toThrow(); + expect(() => handler.testTimeout('', vi.fn())).toThrow( + 'without a name', + ); }); it('should throw error without callback', () => { - expect(() => handler.testTimeout('test', null as any)).toThrow(); + expect(() => handler.testTimeout('test', null as any)).toThrow( + 'without a callback', + ); }); it('should set timer to null after execution', () => { @@ -135,6 +139,21 @@ describe('AsyncHandler', () => { vi.advanceTimersByTime(100); expect(handler.getTimers()['test']).toBeNull(); }); + + it('keeps the handle of a callback that reschedules itself', () => { + const callback = vi.fn(() => { + if (callback.mock.calls.length < 2) { + handler.testTimeout('poll', callback, 100); + } + }); + handler.testTimeout('poll', callback, 100); + vi.advanceTimersByTime(100); + expect(handler.getTimers()['poll']).not.toBeNull(); + // The rescheduled timer can still be cleared by name + handler.testClearTimeout('poll'); + vi.advanceTimersByTime(100); + expect(callback).toHaveBeenCalledTimes(1); + }); }); describe('clearTimeout', () => { @@ -186,11 +205,15 @@ describe('AsyncHandler', () => { }); it('should throw error without name', () => { - expect(() => handler.testInterval('', vi.fn())).toThrow(); + expect(() => handler.testInterval('', vi.fn())).toThrow( + 'without a name', + ); }); it('should throw error without callback', () => { - expect(() => handler.testInterval('test', null as any)).toThrow(); + expect(() => handler.testInterval('test', null as any)).toThrow( + 'without a callback', + ); }); }); diff --git a/src/tests/common/signals.spec.ts b/src/tests/common/signals.spec.ts index 23fc28a3d..94aecfee0 100644 --- a/src/tests/common/signals.spec.ts +++ b/src/tests/common/signals.spec.ts @@ -30,6 +30,20 @@ describe('signals.ts utilities', () => { value.set('hello'); expect(await promise).toBe('hello'); }); + + it('rejects when no matching value arrives in time', async () => { + vi.useFakeTimers(); + try { + const value = signal(false); + const promise = waitForSignalValue(value, Boolean, 50, 1000); + const result = expect(promise).rejects.toThrow('Timed out'); + await vi.advanceTimersByTimeAsync(1000); + await result; + expect(vi.getTimerCount()).toBe(0); + } finally { + vi.useRealTimers(); + } + }); }); }); diff --git a/src/tests/common/user-state.spec.ts b/src/tests/common/user-state.spec.ts index 9e9ca88bd..c6b54bf5b 100644 --- a/src/tests/common/user-state.spec.ts +++ b/src/tests/common/user-state.spec.ts @@ -1,62 +1,24 @@ -import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'; - -vi.mock('@placeos/ts-client', () => ({ - PlaceUser: class { - id = ''; - }, - showUser: vi.fn(), -})); +import { PlaceUser } from '@placeos/ts-client'; +import { describe, expect, it, vi } from 'vitest'; +import { + current_user, + currentUser, + setCurrentUser, +} from '../../app/common/user-state'; + +vi.mock('@placeos/ts-client', async () => + vi.importActual('@placeos/ts-client/dist/index.es.js'), +); describe('current user', () => { - beforeEach(() => { - vi.useFakeTimers(); - vi.resetModules(); - }); - - afterEach(() => { - vi.clearAllTimers(); - vi.useRealTimers(); - vi.resetAllMocks(); - }); - - it('returns a stable empty user until the current user loads', async () => { - const { current_user, currentUser } = await import( - '../../app/common/user-state' - ); - + it('returns a stable empty user until the user is published', () => { expect(current_user()).toBeNull(); expect(currentUser().id).toBe(''); expect(currentUser()).toBe(currentUser()); - }); - - it('publishes the loaded user and stops polling', async () => { - const { PlaceUser, showUser } = await import('@placeos/ts-client'); - const user = new PlaceUser(); - vi.mocked(showUser).mockResolvedValue(user); - const { current_user, currentUser } = await import( - '../../app/common/user-state' - ); - await vi.advanceTimersByTimeAsync(11_000); - - expect(showUser).toHaveBeenCalledExactlyOnceWith('current'); + const user = new PlaceUser({ id: 'user-1' }); + setCurrentUser(user); expect(current_user()).toBe(user); expect(currentUser()).toBe(user); - expect(vi.getTimerCount()).toBe(0); - }); - - it('stops after ten failed requests and keeps the empty user', async () => { - const { showUser } = await import('@placeos/ts-client'); - vi.mocked(showUser).mockRejectedValue(new Error('Unavailable')); - const { current_user, currentUser } = await import( - '../../app/common/user-state' - ); - - await vi.advanceTimersByTimeAsync(11_000); - - expect(showUser).toHaveBeenCalledTimes(10); - expect(current_user()).toBeNull(); - expect(currentUser().id).toBe(''); - expect(vi.getTimerCount()).toBe(0); }); }); From 4133324684a219a3571b2f7f6f6f5ae51a3e3213 Mon Sep 17 00:00:00 2001 From: Alex Sorafumo Date: Wed, 30 Sep 2026 11:15:16 +1000 Subject: [PATCH 6/9] fix: use signals where async work updates the view With zoneless change detection, a plain field set after an await does not re-render. - Trigger comparison form: status variable lists are signals, and a failed module query shows an error. - System trigger form: loading is a signal, and it resets when saving the trigger settings fails. The failure no longer rethrows into the template handler. - Object list field: emit new objects and arrays instead of mutating the parent's objects through ngModel. The delete button is type="button" so it cannot submit the parent form. Co-Authored-By: Claude Opus 5.5 (1M context) --- src/app/systems/system-state.service.ts | 15 ++-- .../systems/system-trigger-form.component.ts | 6 +- .../object-list-field.component.ts | 36 ++++++---- .../comparison-form.component.ts | 70 +++++++++---------- 4 files changed, 65 insertions(+), 62 deletions(-) diff --git a/src/app/systems/system-state.service.ts b/src/app/systems/system-state.service.ts index 7c1bf1c5a..5e3b25777 100644 --- a/src/app/systems/system-state.service.ts +++ b/src/app/systems/system-state.service.ts @@ -41,12 +41,7 @@ import { describeError } from '../common/errors'; import { ActiveItemService } from '../common/item.service'; import { notifyError, notifySuccess } from '../common/notifications'; import { waitForEvent } from '../common/signals'; -import { - DialogEvent, - FormModalComponent, - HashMap, - Identity, -} from '../common/types'; +import { DialogEvent, HashMap, Identity } from '../common/types'; import { ConfirmModalData, openConfirmModal, @@ -463,8 +458,7 @@ export class SystemStateService extends AsyncHandler { external_save: true, }, }); - const instance = - ref.componentInstance as unknown as FormModalComponent; + const instance = ref.componentInstance; const details = await Promise.race([ waitForEvent( instance.event, @@ -473,7 +467,7 @@ export class SystemStateService extends AsyncHandler { waitForEvent(ref.afterClosed()), ]); if (details?.reason !== 'action') return; - instance.loading = 'Saving trigger settings...'; + instance.loading.set('Saving trigger settings...'); const url = `${apiEndpoint()}/systems/${ this.active_item.id @@ -482,8 +476,9 @@ export class SystemStateService extends AsyncHandler { notifyError( `Error updating trigger settings. Error: ${describeError(err)}`, ); - throw err; + return null; }); + instance.loading.set(''); ref.close(); if (!trig) return trigger; notifySuccess(`Successfully updated trigger settings.`); diff --git a/src/app/systems/system-trigger-form.component.ts b/src/app/systems/system-trigger-form.component.ts index 65c8b756f..a21ecef47 100644 --- a/src/app/systems/system-trigger-form.component.ts +++ b/src/app/systems/system-trigger-form.component.ts @@ -35,7 +35,7 @@ import { TranslatePipe } from '../ui/translate.pipe'; template: ` @if (form) { @@ -204,7 +204,7 @@ export class SystemTriggerFormComponent extends AsyncHandler implements OnInit { generateTriggerSettingsFormModel(this._data.item), ); public readonly form = form(this.formModel); - public loading: string; + public readonly loading = signal(''); public heading = i18n(`TRIGGERS.${this._data.item.id ? 'EDIT' : 'NEW'}`); public readonly trigger_state = this.formModel.asReadonly(); @@ -245,7 +245,7 @@ export class SystemTriggerFormComponent extends AsyncHandler implements OnInit { public async submit(): Promise { // Caller sets `loading` while it saves. Ignore repeat submits. - if (this.loading) return; + if (this.loading()) return; await submit(this.form, async () => undefined); if (this.form().invalid()) { return notifyError( diff --git a/src/app/ui/custom-fields/object-list-field.component.ts b/src/app/ui/custom-fields/object-list-field.component.ts index 887674d18..1cdb1d758 100644 --- a/src/app/ui/custom-fields/object-list-field.component.ts +++ b/src/app/ui/custom-fields/object-list-field.component.ts @@ -28,7 +28,7 @@ import { TranslatePipe } from '../translate.pipe';
} - @for (item of active_list(); track item) { + @for (item of active_list(); track $index) {
@for (field of fields(); track field) {
@@ -38,9 +38,9 @@ import { TranslatePipe } from '../translate.pipe'; [name]="field" [placeholder]="field" [disabled]="disabled()" - [(ngModel)]="item[field]" + [ngModel]="item[field]" (ngModelChange)=" - setValue(active_list()) + updateField($index, field, $event) " /> @@ -50,9 +50,10 @@ import { TranslatePipe } from '../translate.pipe'; type="button" icon matRipple + type="button" class="border-error text-error h-12 w-12 rounded-sm border" [disabled]="disabled()" - (click)="removeRow(item)" + (click)="removeRow($index)" > delete @@ -157,16 +158,27 @@ export class ObjectListFieldComponent /** * Remove item from the active list - * @param item Item to remove + * @param index Index of the item to remove */ - public removeRow(item: HashMap) { + public removeRow(index: number) { if (this.disabled()) return; - const index = this.active_list().indexOf(item); - if (index >= 0) { - this.active_list.update((list) => - list.filter((_, item_index) => item_index !== index), - ); - } + this.active_list.update((list) => + list.filter((_, item_index) => item_index !== index), + ); + this.setValue(this.active_list()); + } + + /** + * Set a field on one item. Emits a new list with a new item, so the + * parent's objects are never changed in place. + */ + public updateField(index: number, field: string, value: string) { + if (this.disabled()) return; + this.active_list.update((list) => + list.map((item, item_index) => + item_index === index ? { ...item, [field]: value } : item, + ), + ); this.setValue(this.active_list()); } diff --git a/src/app/ui/forms/trigger-condition-form/comparison-form.component.ts b/src/app/ui/forms/trigger-condition-form/comparison-form.component.ts index 2e68894dc..4f300cf92 100644 --- a/src/app/ui/forms/trigger-condition-form/comparison-form.component.ts +++ b/src/app/ui/forms/trigger-condition-form/comparison-form.component.ts @@ -23,6 +23,7 @@ import { MatFormFieldModule } from '@angular/material/form-field'; import { MatInputModule } from '@angular/material/input'; import { MatSelectModule } from '@angular/material/select'; import { calculateModuleIndex } from '../../../common/api'; +import { describeError } from '../../../common/errors'; import { i18n } from '../../../common/locale.service'; import { notifyError } from '../../../common/notifications'; import { Identity } from '../../../common/types'; @@ -155,7 +156,7 @@ import { TranslatePipe } from '../../translate.pipe';
} - @if (this[side + '_status_variables']?.length) { + @if (statusVariables(side)().length) {
}
- @if ( - this[side + '_status_variables'] && - this[side + '_status_variables'].length - ) { + @if (statusVariables(side)().length) {
@@ -313,8 +312,15 @@ export class ZoneAboutComponent extends AsyncHandler { }); } + /** Loads the parent zone name. The view shows the parent ID if this fails */ private async loadParent(parent_id: string) { - const zone = await showZone(parent_id); + const zone = await showZone(parent_id).catch((err) => { + console.warn( + `Failed to load parent zone ${parent_id}:`, + describeError(err), + ); + return null; + }); if (this.item()?.parent_id === parent_id && zone) this.parent.set(zone); } } diff --git a/src/app/zones/zones-state.service.ts b/src/app/zones/zones-state.service.ts index f5153589c..b96a98eb0 100644 --- a/src/app/zones/zones-state.service.ts +++ b/src/app/zones/zones-state.service.ts @@ -18,8 +18,8 @@ import { updateGroupZone, updateZone, } from '@placeos/ts-client'; -import { describeError } from '../common/errors'; import { escapeHtml, unique } from '../common/general'; +import { describeError, readError } from '../common/errors'; import { ActiveItemService } from '../common/item.service'; import { i18n } from '../common/locale.service'; import { notifyError, notifySuccess } from '../common/notifications'; @@ -261,19 +261,23 @@ export class ZonesStateService { this._dialog, ); if (details.reason !== 'done') return; - const zone = await updateZone(this.active_item.id, { - ...this.active_item, - triggers: this.active_item.triggers.filter((t) => t !== trigger.id), - }).catch((err) => { - details.close(); - notifyError( - `Error removing trigger ${trigger.id} from zone. Error: ${describeError( + let zone: PlaceZone; + try { + zone = await updateZone(this.active_item.id, { + ...this.active_item, + triggers: this.active_item.triggers.filter( + (t) => t !== trigger.id, + ), + }); + } catch (err) { + return notifyError( + `Error removing trigger ${trigger.id} from zone. Error: ${await readError( err, )}`, ); - throw err; - }); - details.close(); + } finally { + details.close(); + } notifySuccess(`Successfully removed trigger from zone.`); if (zone) this._service.replaceItem(zone as unknown as Identity); } From 4864b155d8de185dcde319bcf1dabfce5ddf4672 Mon Sep 17 00:00:00 2001 From: Alex Sorafumo Date: Wed, 30 Sep 2026 12:24:09 +1000 Subject: [PATCH 9/9] fix: notify and return instead of rethrowing after error toasts - Metadata delete, execute method, module power toggle, signage AI provider save and storage provider save no longer rethrow into template handlers, so no unhandled rejection reaches the console - Signage AI provider and storage provider save toasts include the API error detail Co-Authored-By: Claude Opus 5.5 (1M context) --- .../signage-ai-provider-modal.component.ts | 16 +++++++------ .../storage-provider-modal.component.ts | 15 +++++++----- src/app/systems/system-state.service.ts | 24 +++++++++---------- .../execute-method-field.component.ts | 5 +++- src/app/ui/metadata-display.component.ts | 17 ++++++------- 5 files changed, 43 insertions(+), 34 deletions(-) diff --git a/src/app/admin/signage-ai/signage-ai-provider-modal.component.ts b/src/app/admin/signage-ai/signage-ai-provider-modal.component.ts index f8b1ba834..c17b12b7b 100644 --- a/src/app/admin/signage-ai/signage-ai-provider-modal.component.ts +++ b/src/app/admin/signage-ai/signage-ai-provider-modal.component.ts @@ -4,6 +4,7 @@ import { MAT_DIALOG_DATA, MatDialogRef } from '@angular/material/dialog'; import { MatFormFieldModule } from '@angular/material/form-field'; import { MatInputModule } from '@angular/material/input'; import { MatSelectModule } from '@angular/material/select'; +import { readError } from '../../common/errors'; import { i18n } from '../../common/locale.service'; import { notifyError, notifySuccess } from '../../common/notifications'; import { FullscreenModalShellComponent } from '../../ui/fullscreen-modal-shell.component'; @@ -300,15 +301,16 @@ export class SignageAIProviderModalComponent { // an edit that leaves the boxes empty keeps the stored credentials if (Object.keys(credentials).length) body.credentials = credentials; - await saveSignageAIProvider(body).catch((error) => { - notifyError(i18n('ADMIN.AI_PROVIDER_SAVE_ERROR')); + try { + await saveSignageAIProvider(body); + } catch (error) { + return notifyError( + `${i18n('ADMIN.AI_PROVIDER_SAVE_ERROR')}: ${await readError(error)}`, + ); + } finally { this.loading.set(''); this._dialog_ref.disableClose = false; - throw error; - }); - - this.loading.set(''); - this._dialog_ref.disableClose = false; + } notifySuccess(i18n('ADMIN.AI_PROVIDER_SAVE_SUCCESS')); this._dialog_ref.close(true); } diff --git a/src/app/admin/storage/storage-provider-modal.component.ts b/src/app/admin/storage/storage-provider-modal.component.ts index 6b6ea7a2f..c04441c72 100644 --- a/src/app/admin/storage/storage-provider-modal.component.ts +++ b/src/app/admin/storage/storage-provider-modal.component.ts @@ -4,6 +4,7 @@ import { MAT_DIALOG_DATA, MatDialogRef } from '@angular/material/dialog'; import { MatFormFieldModule } from '@angular/material/form-field'; import { MatInputModule } from '@angular/material/input'; import { MatSelectModule } from '@angular/material/select'; +import { readError } from '../../common/errors'; import { i18n } from '../../common/locale.service'; import { notifyError, notifySuccess } from '../../common/notifications'; import { FullscreenModalShellComponent } from '../../ui/fullscreen-modal-shell.component'; @@ -262,14 +263,16 @@ export class StorageProviderModalComponent { delete (details as PlaceStorage & { access_secret?: unknown }) .access_secret; } - await saveStorage(details).catch((e) => { - notifyError(i18n('ADMIN.STORAGE_SAVE_ERROR')); + try { + await saveStorage(details); + } catch (err) { + return notifyError( + `${i18n('ADMIN.STORAGE_SAVE_ERROR')} Error: ${await readError(err)}`, + ); + } finally { this.loading.set(''); this._dialog_ref.disableClose = false; - throw e; - }); - this.loading.set(''); - this._dialog_ref.disableClose = false; + } notifySuccess(i18n('ADMIN.STORAGE_SAVE_SUCCESS')); this._dialog_ref.close(); } diff --git a/src/app/systems/system-state.service.ts b/src/app/systems/system-state.service.ts index d8a11d251..bde3f1592 100644 --- a/src/app/systems/system-state.service.ts +++ b/src/app/systems/system-state.service.ts @@ -742,20 +742,20 @@ export class SystemStateService extends AsyncHandler { */ public async toggleModulePower(device: PlaceModule) { const method = device.running ? stopModule : startModule; - await method(device.id).catch((err) => { + try { + await method(device.id); + } catch (err) { if (typeof err === 'string' && err.length < 64) { - notifyError(err); - } else { - notifyError( - `Failed to ${ - device.running ? 'stop' : 'start' - } module '${device.id}'.\nView Error?`, - 'View', - () => this.viewDetails(err), - ); + return notifyError(err); } - throw err; - }); + return notifyError( + `Failed to ${ + device.running ? 'stop' : 'start' + } module '${device.id}'.\nView Error?`, + 'View', + () => this.viewDetails(err), + ); + } notifySuccess( `Module successfully ${device.running ? 'stopped' : 'started'}`, ); diff --git a/src/app/ui/custom-fields/system-exec/execute-method-field.component.ts b/src/app/ui/custom-fields/system-exec/execute-method-field.component.ts index 0f1b0509a..3d05d5cfc 100644 --- a/src/app/ui/custom-fields/system-exec/execute-method-field.component.ts +++ b/src/app/ui/custom-fields/system-exec/execute-method-field.component.ts @@ -217,6 +217,7 @@ export class ExecuteMethodFieldComponent implements ControlValueAccessor { this.loading.set(true); this.arguments.set(this.arguments() || {}); const method = this.zone() ? executeOnZone : executeOnSystem; + let failed = false; const result = await method( this.zone() || this.system().id, (this.fn() as unknown as { name: string }).name, @@ -250,8 +251,10 @@ export class ExecuteMethodFieldComponent implements ControlValueAccessor { ); } this.loading.set(false); - throw err; + failed = true; + return null; }); + if (failed) return; notifySuccess( 'Command successful executed.\nView Response?', 'View', diff --git a/src/app/ui/metadata-display.component.ts b/src/app/ui/metadata-display.component.ts index 958e79b83..6e83618a7 100644 --- a/src/app/ui/metadata-display.component.ts +++ b/src/app/ui/metadata-display.component.ts @@ -25,7 +25,7 @@ import { VERSION } from '../../env/version'; import { escapeHtml } from '../common/general'; // import { SchemaStateService } from '../admin/schema-state.service'; import { AsyncHandler } from '../common/async-handler.class'; -import { describeError } from '../common/errors'; +import { describeError, readError } from '../common/errors'; import { notifyError, notifySuccess } from '../common/notifications'; import { HashMap } from '../common/types'; import { currentUser } from '../common/user-state'; @@ -336,16 +336,17 @@ export class MetadataDisplayComponent this._dialog, ); if (result.reason !== 'done') return; - await removeMetadata(this.item().id, { name: field }).catch((err) => { - result.close(); - notifyError( - `Error removing old "${field}" metadata. Error: ${describeError( + try { + await removeMetadata(this.item().id, { name: field }); + } catch (err) { + return notifyError( + `Error removing old "${field}" metadata. Error: ${await readError( err, )}`, ); - throw err; - }); - result.close(); + } finally { + result.close(); + } notifySuccess(`Successfully removed "${field}" metadata.`); this.metadata.set( this.metadata().filter((prop) => prop && prop.name !== field),