mirror of
https://github.com/bulwarkmail/webmail.git
synced 2026-10-31 00:12:39 +00:00
fix: report calendar clear failures instead of counting zero #434
This commit is contained in:
@@ -1539,8 +1539,9 @@ export function CalendarApp({ linkSegments }: CalendarAppProps = {}) {
|
||||
try {
|
||||
const count = await clearCalendarEvents(client, cal.id);
|
||||
toast.success(tMgmt("events_cleared", { count }));
|
||||
} catch {
|
||||
toast.error(tMgmt("error_clear"));
|
||||
} catch (err) {
|
||||
// The store rethrows the server's reason (#434).
|
||||
toast.error(err instanceof Error && err.message ? err.message : tMgmt("error_clear"));
|
||||
}
|
||||
} : undefined}
|
||||
onDeleteCalendar={client ? async (cal: Calendar) => {
|
||||
|
||||
@@ -0,0 +1,127 @@
|
||||
import { describe, it, expect, vi, beforeEach, afterEach } from 'vitest';
|
||||
import { JMAPClient } from '../jmap/client';
|
||||
|
||||
// #434: batchDeleteCalendarEvents ignored a method-level error and dropped the
|
||||
// notDestroyed details, so "Clear events" reported "0 events cleared" with no
|
||||
// error. The same for batchCreateCalendarEvents' notCreated on import.
|
||||
|
||||
function makeSession() {
|
||||
return {
|
||||
capabilities: {
|
||||
'urn:ietf:params:jmap:core': {},
|
||||
'urn:ietf:params:jmap:calendars': {},
|
||||
},
|
||||
accounts: {
|
||||
'acct-1': {
|
||||
name: 'test',
|
||||
isPersonal: true,
|
||||
accountCapabilities: { 'urn:ietf:params:jmap:calendars': {} },
|
||||
},
|
||||
},
|
||||
primaryAccounts: {
|
||||
'urn:ietf:params:jmap:mail': 'acct-1',
|
||||
'urn:ietf:params:jmap:calendars': 'acct-1',
|
||||
},
|
||||
apiUrl: 'https://mail.example.com/jmap/api',
|
||||
downloadUrl: 'https://mail.example.com/jmap/download/{accountId}/{blobId}/{name}',
|
||||
uploadUrl: 'https://mail.example.com/jmap/upload/{accountId}/',
|
||||
eventSourceUrl: 'https://mail.example.com/jmap/eventsource',
|
||||
};
|
||||
}
|
||||
|
||||
function jsonResponse(body: unknown): Response {
|
||||
return new Response(JSON.stringify(body), {
|
||||
status: 200,
|
||||
headers: { 'Content-Type': 'application/json' },
|
||||
});
|
||||
}
|
||||
|
||||
describe('calendar batch failures surface (#434)', () => {
|
||||
let fetchSpy: ReturnType<typeof vi.spyOn>;
|
||||
|
||||
beforeEach(() => {
|
||||
fetchSpy = vi.spyOn(globalThis, 'fetch');
|
||||
});
|
||||
|
||||
afterEach(() => {
|
||||
fetchSpy.mockRestore();
|
||||
});
|
||||
|
||||
async function connectedClient(): Promise<JMAPClient> {
|
||||
fetchSpy.mockResolvedValueOnce(jsonResponse(makeSession()));
|
||||
const client = JMAPClient.withBearer('https://mail.example.com', 'token123', 'user@test.com');
|
||||
await client.connect();
|
||||
fetchSpy.mockReset();
|
||||
return client;
|
||||
}
|
||||
|
||||
it('batchDeleteCalendarEvents rejects with the server reason when nothing was destroyed', async () => {
|
||||
const client = await connectedClient();
|
||||
fetchSpy.mockResolvedValueOnce(
|
||||
jsonResponse({
|
||||
methodResponses: [
|
||||
['CalendarEvent/set', {
|
||||
destroyed: null,
|
||||
notDestroyed: { ev1: { type: 'forbidden', description: 'You do not own this calendar' } },
|
||||
}, '0'],
|
||||
],
|
||||
}),
|
||||
);
|
||||
|
||||
await expect(client.batchDeleteCalendarEvents(['ev1'])).rejects.toThrow(/You do not own this calendar/);
|
||||
});
|
||||
|
||||
it('batchDeleteCalendarEvents rejects on a method-level error', async () => {
|
||||
const client = await connectedClient();
|
||||
fetchSpy.mockResolvedValueOnce(
|
||||
jsonResponse({
|
||||
methodResponses: [['error', { type: 'accountNotFound', description: 'Account not found' }, '0']],
|
||||
}),
|
||||
);
|
||||
|
||||
await expect(client.batchDeleteCalendarEvents(['ev1'])).rejects.toThrow(/Account not found/);
|
||||
});
|
||||
|
||||
it('batchDeleteCalendarEvents returns the notDestroyed map for partial failures', async () => {
|
||||
const client = await connectedClient();
|
||||
fetchSpy.mockResolvedValueOnce(
|
||||
jsonResponse({
|
||||
methodResponses: [
|
||||
['CalendarEvent/set', {
|
||||
destroyed: ['ev1'],
|
||||
notDestroyed: { ev2: { type: 'notFound', description: 'Gone' } },
|
||||
}, '0'],
|
||||
],
|
||||
}),
|
||||
);
|
||||
|
||||
const result = await client.batchDeleteCalendarEvents(['ev1', 'ev2']);
|
||||
expect(result.destroyed).toEqual(['ev1']);
|
||||
expect(result.notDestroyed).toEqual({ ev2: { type: 'notFound', description: 'Gone' } });
|
||||
});
|
||||
|
||||
it('batchCreateCalendarEvents rejects on a method-level error and reports notCreated otherwise', async () => {
|
||||
const client = await connectedClient();
|
||||
fetchSpy.mockResolvedValueOnce(
|
||||
jsonResponse({
|
||||
methodResponses: [['error', { type: 'invalidArguments', description: 'Unknown calendar' }, '0']],
|
||||
}),
|
||||
);
|
||||
await expect(client.batchCreateCalendarEvents([{ title: 'A' }])).rejects.toThrow(/Unknown calendar/);
|
||||
|
||||
fetchSpy.mockResolvedValueOnce(
|
||||
jsonResponse({
|
||||
methodResponses: [
|
||||
['CalendarEvent/set', {
|
||||
created: null,
|
||||
notCreated: { 'new-0': { type: 'invalidProperties', description: 'Missing start' } },
|
||||
}, '0'],
|
||||
],
|
||||
}),
|
||||
);
|
||||
const result = await client.batchCreateCalendarEvents([{ title: 'A' }]);
|
||||
expect(result.created).toEqual([]);
|
||||
expect(result.failed).toEqual(['new-0']);
|
||||
expect(result.notCreated['new-0']).toMatchObject({ description: 'Missing start' });
|
||||
});
|
||||
});
|
||||
@@ -915,13 +915,13 @@ export class DemoJMAPClient implements IJMAPClient {
|
||||
return full;
|
||||
}
|
||||
|
||||
async batchCreateCalendarEvents(events: Partial<CalendarEvent>[]): Promise<{ created: CalendarEvent[]; failed: string[] }> {
|
||||
async batchCreateCalendarEvents(events: Partial<CalendarEvent>[]): Promise<{ created: CalendarEvent[]; failed: string[]; notCreated: Record<string, { type?: string; description?: string }> }> {
|
||||
const created: CalendarEvent[] = [];
|
||||
for (const event of events) {
|
||||
const full = await this.createCalendarEvent(event);
|
||||
created.push(full);
|
||||
}
|
||||
return { created, failed: [] };
|
||||
return { created, failed: [], notCreated: {} };
|
||||
}
|
||||
|
||||
async updateCalendarEvent(eventId: string, updates: Partial<CalendarEvent>): Promise<void> {
|
||||
@@ -934,10 +934,10 @@ export class DemoJMAPClient implements IJMAPClient {
|
||||
this.data.calendarEvents = this.data.calendarEvents.filter(e => e.id !== eventId);
|
||||
}
|
||||
|
||||
async batchDeleteCalendarEvents(eventIds: string[]): Promise<{ destroyed: string[]; notDestroyed: string[] }> {
|
||||
async batchDeleteCalendarEvents(eventIds: string[]): Promise<{ destroyed: string[]; notDestroyed: Record<string, { type?: string; description?: string }> }> {
|
||||
const idSet = new Set(eventIds);
|
||||
this.data.calendarEvents = this.data.calendarEvents.filter(e => !idSet.has(e.id));
|
||||
return { destroyed: eventIds, notDestroyed: [] };
|
||||
return { destroyed: eventIds, notDestroyed: {} };
|
||||
}
|
||||
|
||||
async queryCalendarEvents(filter: CalendarEventFilter): Promise<CalendarEvent[]> {
|
||||
|
||||
@@ -405,7 +405,7 @@ export interface IJMAPClient {
|
||||
getCalendarEvents(calendarIds?: string[], targetAccountId?: string): Promise<CalendarEvent[]>;
|
||||
getCalendarEvent(id: string, targetAccountId?: string): Promise<CalendarEvent | null>;
|
||||
createCalendarEvent(event: Partial<CalendarEvent>, sendSchedulingMessages?: boolean, targetAccountId?: string): Promise<CalendarEvent>;
|
||||
batchCreateCalendarEvents(events: Partial<CalendarEvent>[], targetAccountId?: string): Promise<{ created: CalendarEvent[]; failed: string[] }>;
|
||||
batchCreateCalendarEvents(events: Partial<CalendarEvent>[], targetAccountId?: string): Promise<{ created: CalendarEvent[]; failed: string[]; notCreated: Record<string, { type?: string; description?: string }> }>;
|
||||
updateCalendarEvent(
|
||||
eventId: string,
|
||||
updates: Partial<CalendarEvent>,
|
||||
@@ -413,7 +413,7 @@ export interface IJMAPClient {
|
||||
targetAccountId?: string,
|
||||
): Promise<void>;
|
||||
deleteCalendarEvent(eventId: string, sendSchedulingMessages?: boolean, targetAccountId?: string): Promise<void>;
|
||||
batchDeleteCalendarEvents(eventIds: string[], targetAccountId?: string): Promise<{ destroyed: string[]; notDestroyed: string[] }>;
|
||||
batchDeleteCalendarEvents(eventIds: string[], targetAccountId?: string): Promise<{ destroyed: string[]; notDestroyed: Record<string, { type?: string; description?: string }> }>;
|
||||
queryCalendarEvents(filter: CalendarEventFilter, sort?: Array<{ property: string; isAscending: boolean }>, limit?: number, targetAccountId?: string): Promise<CalendarEvent[]>;
|
||||
queryAllCalendarEvents(filter: CalendarEventFilter, sort?: Array<{ property: string; isAscending: boolean }>, limit?: number): Promise<CalendarEvent[]>;
|
||||
parseCalendarEvents(accountId: string, blobId: string): Promise<Partial<CalendarEvent>[]>;
|
||||
|
||||
+51
-9
@@ -173,6 +173,12 @@ interface JMAPResponse {
|
||||
methodResponses: Array<[string, JMAPResponseResult, string]>;
|
||||
}
|
||||
|
||||
/** Per-id failure entry of a CalendarEvent/set (`notCreated` / `notDestroyed`). */
|
||||
export interface CalendarSetError {
|
||||
type?: string;
|
||||
description?: string;
|
||||
}
|
||||
|
||||
const DEFAULT_MAILBOX_RIGHTS = {
|
||||
mayReadItems: true,
|
||||
mayAddItems: true,
|
||||
@@ -6534,8 +6540,8 @@ export class JMAPClient implements IJMAPClient {
|
||||
async batchCreateCalendarEvents(
|
||||
events: Partial<CalendarEvent>[],
|
||||
targetAccountId?: string,
|
||||
): Promise<{ created: CalendarEvent[]; failed: string[] }> {
|
||||
if (events.length === 0) return { created: [], failed: [] };
|
||||
): Promise<{ created: CalendarEvent[]; failed: string[]; notCreated: Record<string, CalendarSetError> }> {
|
||||
if (events.length === 0) return { created: [], failed: [], notCreated: {} };
|
||||
|
||||
const accountId = targetAccountId || this.getCalendarsAccountId();
|
||||
|
||||
@@ -6543,6 +6549,7 @@ export class JMAPClient implements IJMAPClient {
|
||||
|
||||
const createdIds: string[] = [];
|
||||
const failed: string[] = [];
|
||||
const notCreated: Record<string, CalendarSetError> = {};
|
||||
const indexed = events.map((event, index) => ({ event, index }));
|
||||
|
||||
for (const batch of batched(indexed, this.getMaxObjectsInSet())) {
|
||||
@@ -6564,6 +6571,10 @@ export class JMAPClient implements IJMAPClient {
|
||||
["CalendarEvent/set", { accountId, sendSchedulingMessages: false, create: createMap }, "0"]
|
||||
], this.calendarUsing());
|
||||
|
||||
// A method-level error (unknown calendar, missing capability, …) is
|
||||
// not a per-event failure: nothing was created, say so. (#434)
|
||||
this.throwOnCalendarSetError(response, 'create calendar events');
|
||||
|
||||
if (response.methodResponses?.[0]?.[0] === "CalendarEvent/set") {
|
||||
const result = response.methodResponses[0][1];
|
||||
for (const { index } of batch) {
|
||||
@@ -6573,13 +6584,14 @@ export class JMAPClient implements IJMAPClient {
|
||||
} else if (result.notCreated?.[key]) {
|
||||
debug.warn('calendar', `CalendarEvent/batchCreate failed for ${key}`, result.notCreated[key]);
|
||||
failed.push(key);
|
||||
notCreated[key] = result.notCreated[key];
|
||||
}
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
if (createdIds.length === 0) {
|
||||
return { created: [], failed };
|
||||
return { created: [], failed, notCreated };
|
||||
}
|
||||
|
||||
// Fetch the created events back for their server-assigned properties
|
||||
@@ -6608,7 +6620,7 @@ export class JMAPClient implements IJMAPClient {
|
||||
failed: failed.length,
|
||||
});
|
||||
|
||||
return { created: createdEvents, failed };
|
||||
return { created: createdEvents, failed, notCreated };
|
||||
}
|
||||
|
||||
async updateCalendarEvent(
|
||||
@@ -6746,22 +6758,52 @@ export class JMAPClient implements IJMAPClient {
|
||||
throw new Error("Failed to delete calendar event");
|
||||
}
|
||||
|
||||
async batchDeleteCalendarEvents(eventIds: string[], targetAccountId?: string): Promise<{ destroyed: string[]; notDestroyed: string[] }> {
|
||||
if (eventIds.length === 0) return { destroyed: [], notDestroyed: [] };
|
||||
/**
|
||||
* Reject a CalendarEvent/set response that came back as a method-level
|
||||
* error. Without this the callers counted the (empty) results and
|
||||
* reported "0 events cleared" with no error at all. (#434)
|
||||
*/
|
||||
private throwOnCalendarSetError(response: JMAPResponse, action: string): void {
|
||||
const [name, result] = response.methodResponses?.[0] ?? [];
|
||||
if (name === "error") {
|
||||
throw new Error(`Failed to ${action}: ${result?.description || result?.type || 'unknown error'}`);
|
||||
}
|
||||
}
|
||||
|
||||
/**
|
||||
* Destroy events in batches. Per-id failures come back in `notDestroyed`
|
||||
* (with the server's type/description) so the caller can report them;
|
||||
* a method-level error, or a batch in which nothing at all could be
|
||||
* destroyed, rejects with the first description. (#434)
|
||||
*/
|
||||
async batchDeleteCalendarEvents(eventIds: string[], targetAccountId?: string): Promise<{ destroyed: string[]; notDestroyed: Record<string, CalendarSetError> }> {
|
||||
if (eventIds.length === 0) return { destroyed: [], notDestroyed: {} };
|
||||
|
||||
const accountId = targetAccountId || this.getCalendarsAccountId();
|
||||
const destroyed: string[] = [];
|
||||
const notDestroyed: string[] = [];
|
||||
const notDestroyed: Record<string, CalendarSetError> = {};
|
||||
|
||||
for (const batch of batched(eventIds, this.getMaxObjectsInSet())) {
|
||||
const response = await this.request([
|
||||
["CalendarEvent/set", { accountId, destroy: batch }, "0"]
|
||||
], this.calendarUsing());
|
||||
|
||||
this.throwOnCalendarSetError(response, 'delete calendar events');
|
||||
|
||||
let destroyedInBatch = 0;
|
||||
if (response.methodResponses?.[0]?.[0] === "CalendarEvent/set") {
|
||||
const result = response.methodResponses[0][1];
|
||||
if (result.destroyed) destroyed.push(...result.destroyed);
|
||||
if (result.notDestroyed) notDestroyed.push(...Object.keys(result.notDestroyed));
|
||||
if (result.destroyed) {
|
||||
destroyed.push(...result.destroyed);
|
||||
destroyedInBatch = result.destroyed.length;
|
||||
}
|
||||
if (result.notDestroyed) Object.assign(notDestroyed, result.notDestroyed);
|
||||
}
|
||||
if (destroyedInBatch === 0 && batch.length > 0) {
|
||||
const first = Object.values(notDestroyed)[0];
|
||||
throw new Error(
|
||||
`Failed to delete calendar events: ${first?.description || first?.type || 'server refused the request'}`,
|
||||
);
|
||||
}
|
||||
}
|
||||
|
||||
|
||||
@@ -1005,10 +1005,13 @@ export const useCalendarStore = create<CalendarStore>()(
|
||||
for (let i = 0; i < prepared.length; i += BATCH_SIZE) {
|
||||
const batch = prepared.slice(i, i + BATCH_SIZE);
|
||||
try {
|
||||
const { created, failed } = await client.batchCreateCalendarEvents(batch, targetAccountId);
|
||||
const { created, failed, notCreated } = await client.batchCreateCalendarEvents(batch, targetAccountId);
|
||||
imported += created.length;
|
||||
if (failed.length > 0) {
|
||||
debug.warn('calendar', `Import batch ${i / BATCH_SIZE + 1}: ${failed.length} events failed`);
|
||||
const first = Object.values(notCreated)[0];
|
||||
const reason = first?.description || first?.type || 'unknown error';
|
||||
debug.warn('calendar', `Import batch ${i / BATCH_SIZE + 1}: ${failed.length} events failed (${reason})`);
|
||||
set({ error: `${failed.length} event(s) could not be imported: ${reason}` });
|
||||
}
|
||||
} catch (error) {
|
||||
debug.error(`Import batch ${i / BATCH_SIZE + 1} failed:`, error);
|
||||
@@ -1185,6 +1188,7 @@ export const useCalendarStore = create<CalendarStore>()(
|
||||
const targetAccountId = cal?.accountId;
|
||||
client = resolveAccountClient(client, cal?.localAccountId);
|
||||
let totalRemoved = 0;
|
||||
let firstRefusalMessage = '';
|
||||
// Loop to handle pagination (getCalendarEvents has a 1000 limit)
|
||||
let hasMore = true;
|
||||
while (hasMore) {
|
||||
@@ -1211,8 +1215,14 @@ export const useCalendarStore = create<CalendarStore>()(
|
||||
|
||||
let removedThisPass = 0;
|
||||
if (idsToDelete.length > 0) {
|
||||
const { destroyed } = await client.batchDeleteCalendarEvents(idsToDelete, targetAccountId);
|
||||
// Rejects on a method-level error or when nothing could be
|
||||
// destroyed; partial refusals are reported once below. (#434)
|
||||
const { destroyed, notDestroyed } = await client.batchDeleteCalendarEvents(idsToDelete, targetAccountId);
|
||||
removedThisPass += destroyed.length;
|
||||
const firstRefusal = Object.values(notDestroyed)[0];
|
||||
if (firstRefusal && !firstRefusalMessage) {
|
||||
firstRefusalMessage = firstRefusal.description || firstRefusal.type || 'server refused the request';
|
||||
}
|
||||
}
|
||||
for (const { id, calendarIds } of eventsToUnlink) {
|
||||
try {
|
||||
@@ -1224,10 +1234,15 @@ export const useCalendarStore = create<CalendarStore>()(
|
||||
}
|
||||
totalRemoved += removedThisPass;
|
||||
|
||||
// If we couldn't remove anything, stop to avoid infinite loop
|
||||
// If we couldn't remove anything, stop to avoid infinite loop -
|
||||
// and say why, instead of reporting "0 events cleared". (#434)
|
||||
if (removedThisPass === 0) {
|
||||
debug.warn('calendar', 'Could not clear any events, stopping. Remaining:', calendarEvents.length);
|
||||
break;
|
||||
throw new Error(
|
||||
firstRefusalMessage
|
||||
? `Failed to clear calendar events: ${firstRefusalMessage}`
|
||||
: 'Failed to clear calendar events: the server did not remove any of them',
|
||||
);
|
||||
}
|
||||
|
||||
// If we got fewer than the limit, we've fetched everything
|
||||
@@ -1240,7 +1255,7 @@ export const useCalendarStore = create<CalendarStore>()(
|
||||
return totalRemoved;
|
||||
} catch (error) {
|
||||
debug.error('Failed to clear calendar events:', error);
|
||||
set({ error: 'Failed to clear calendar events' });
|
||||
set({ error: error instanceof Error ? error.message : 'Failed to clear calendar events' });
|
||||
throw error;
|
||||
}
|
||||
},
|
||||
|
||||
Reference in New Issue
Block a user