Report write failures instead of swallowing them
From the same pre-deploy review. None of the four write calls had an error callback, so a failure closed the row and saved nothing while looking exactly like success. That was survivable when the session was a localStorage value this app controlled; after the cutover a 401 is routine - the session expires, or is ended from another app or another tab - so silence is not. A 401 now says so and goes to the login carrying this page as the return target; everything else says what happened and leaves the edits on screen. getEvents had the same gap, and it is the one that matters during the deploy itself: between the API going out and this bundle following it, the old code renders as signed in and shows an empty table, which reads as "the calendar lost its data" rather than "a deploy is in progress". Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
This commit is contained in:
@@ -3,7 +3,9 @@ import {Subject} from 'rxjs';
|
||||
import {takeUntil} from 'rxjs/operators';
|
||||
import {MatDialog} from '@angular/material/dialog';
|
||||
import {Event} from '../../models/event';
|
||||
import {HttpErrorResponse} from '@angular/common/http';
|
||||
import {ApiService} from '../../services/api.service';
|
||||
import {AdminAuthService} from '../../services/admin-auth.service';
|
||||
import {EventMovePopupComponent} from "../event-move-popup/event-move-popup.component";
|
||||
|
||||
@Component({
|
||||
@@ -71,20 +73,21 @@ export class EventComponent implements OnInit, OnDestroy {
|
||||
}
|
||||
|
||||
if(this.event.eventId === undefined) {
|
||||
this.api.createEvent(this.event).pipe(takeUntil(this.destroy$)).subscribe((res: any) => {
|
||||
console.log(res);
|
||||
|
||||
this.api.createEvent(this.event).pipe(takeUntil(this.destroy$)).subscribe({
|
||||
next: (res: any) => {
|
||||
if(res.eventId) {
|
||||
this.event!.eventId = res.eventId;
|
||||
} else {
|
||||
this.showCreateError = true;
|
||||
return;
|
||||
}
|
||||
},
|
||||
error: this.handleWriteError('The new event')
|
||||
});
|
||||
} else {
|
||||
// Update existing event
|
||||
this.api.updateEvent(this.event).pipe(takeUntil(this.destroy$)).subscribe((res: any) => {
|
||||
console.log(res);
|
||||
this.api.updateEvent(this.event).pipe(takeUntil(this.destroy$)).subscribe({
|
||||
next: () => {},
|
||||
error: this.handleWriteError('Your change')
|
||||
});
|
||||
}
|
||||
}
|
||||
@@ -180,15 +183,48 @@ export class EventComponent implements OnInit, OnDestroy {
|
||||
let deleteConfirmed = window.confirm(`Are you sure you want to delete "${this.event!.name}"? This action cannot be undone.`);
|
||||
|
||||
if(deleteConfirmed && this.event) {
|
||||
this.api.deleteEvent(this.event).pipe(takeUntil(this.destroy$)).subscribe((res: any) => {
|
||||
console.log(res);
|
||||
this.api.deleteEvent(this.event).pipe(takeUntil(this.destroy$)).subscribe({
|
||||
next: (res: any) => {
|
||||
if(res.message) {
|
||||
this.deleteEvent.next(this.event!.eventId);
|
||||
}
|
||||
},
|
||||
error: this.handleWriteError('The deletion')
|
||||
});
|
||||
}
|
||||
}
|
||||
|
||||
/**
|
||||
* What to do when a write fails.
|
||||
*
|
||||
* Before the auth cutover none of the writes here had an error callback, so
|
||||
* a failure was invisible: the row closed, nothing was saved, and the user
|
||||
* had every reason to think it had been. A 401 is now a routine event - the
|
||||
* session expires, or is ended from another app or another tab - so silence
|
||||
* is no longer survivable.
|
||||
*
|
||||
* A 401 means the session is gone, and nothing on this page can be saved
|
||||
* until it comes back, so it goes straight to the login carrying this page
|
||||
* as the return target. Everything else says what happened and leaves the
|
||||
* user where they are, with their edits still on screen.
|
||||
*/
|
||||
private handleWriteError(action: string): (error: HttpErrorResponse) => void {
|
||||
return (error: HttpErrorResponse) => {
|
||||
if (error.status === 401) {
|
||||
window.alert(`Your session has expired, so ${action} was not saved. Signing you in again.`);
|
||||
AdminAuthService.goToLogin();
|
||||
return;
|
||||
}
|
||||
|
||||
if (error.status === 403) {
|
||||
window.alert(`${action} failed: this account no longer has access to the calendar.`);
|
||||
return;
|
||||
}
|
||||
|
||||
window.alert(`${action} failed. Please try again. (${error.status || 'no response from the server'})`);
|
||||
};
|
||||
}
|
||||
|
||||
triggerMove() {
|
||||
if(this.editActive) {
|
||||
window.alert('Please save your changes before moving the event to a different calendar.');
|
||||
@@ -204,11 +240,13 @@ export class EventComponent implements OnInit, OnDestroy {
|
||||
movePopup.afterClosed().pipe(takeUntil(this.destroy$)).subscribe(result => {
|
||||
// If popup is dismissed, undefined will be returned
|
||||
if(result) {
|
||||
this.api.moveEvent(result).pipe(takeUntil(this.destroy$)).subscribe((res: any) => {
|
||||
console.log(res);
|
||||
this.api.moveEvent(result).pipe(takeUntil(this.destroy$)).subscribe({
|
||||
next: () => {
|
||||
// Uses the same interface as delete as from the calendar table perspective it is the same action
|
||||
// as a delete
|
||||
this.deleteEvent.next(result.eventId);
|
||||
},
|
||||
error: this.handleWriteError('The move')
|
||||
});
|
||||
}
|
||||
});
|
||||
|
||||
@@ -78,7 +78,8 @@ export class AdminComponent implements OnInit, OnDestroy {
|
||||
return;
|
||||
}
|
||||
|
||||
this.api.getEvents(this.selectedCalendar).pipe(takeUntil(this.destroy$)).subscribe((events: Event[]): void => {
|
||||
this.api.getEvents(this.selectedCalendar).pipe(takeUntil(this.destroy$)).subscribe({
|
||||
next: (events: Event[]): void => {
|
||||
for (let event of events) {
|
||||
if (event.status !== 'DELETED') {
|
||||
this.events.push({
|
||||
@@ -92,6 +93,18 @@ export class AdminComponent implements OnInit, OnDestroy {
|
||||
}
|
||||
this.filterEvents();
|
||||
this.sortEvents();
|
||||
},
|
||||
// Without this a failed load is indistinguishable from an empty
|
||||
// calendar - which is exactly what the old bundle looks like against
|
||||
// the post-cutover API, and how a deploy in progress gets mistaken for
|
||||
// lost data.
|
||||
error: (error: HttpErrorResponse): void => {
|
||||
if (error.status === 401) {
|
||||
AdminAuthService.goToLogin();
|
||||
return;
|
||||
}
|
||||
this.failure = error.status === 403 ? 'denied' : 'unavailable';
|
||||
}
|
||||
});
|
||||
}
|
||||
|
||||
|
||||
Reference in New Issue
Block a user