From aba2ed1ee3a291f6ef319d531e2039d7ec599272 Mon Sep 17 00:00:00 2001 From: Maurice Date: Sat, 10 Oct 2026 15:33:19 +0200 Subject: [PATCH] refactor(server): places, unsplash, vacay, collections and file downloads raise DomainError, so REST and MCP stop translating results --- .../collections/collections.controller.ts | 10 +-- .../src/nest/collections/collections.mcp.ts | 6 +- .../nest/collections/collections.service.ts | 24 +++--- .../nest/files/files-download.controller.ts | 3 - server/src/nest/files/files.service.ts | 9 ++- server/src/nest/places/places.controller.ts | 6 -- server/src/nest/places/places.mcp.ts | 4 - server/src/nest/places/places.service.ts | 20 ++--- server/src/nest/trips/trips.controller.ts | 3 - server/src/nest/trips/trips.mcp.ts | 5 +- server/src/nest/unsplash/unsplash.service.ts | 7 +- server/src/nest/vacay/vacay.controller.ts | 27 +------ server/src/nest/vacay/vacay.mcp.ts | 19 ++--- server/src/nest/vacay/vacay.service.ts | 79 +++++++++---------- server/tests/helpers/domain-error.ts | 3 +- server/tests/unit/mcp/tools-places.test.ts | 5 +- server/tests/unit/mcp/tools-vacay.test.ts | 5 +- .../unit/nest/collections.controller.test.ts | 5 +- .../unit/nest/collections.service.test.ts | 61 +++++++------- .../tests/unit/nest/files.controller.test.ts | 3 +- server/tests/unit/nest/files.service.test.ts | 13 +-- .../tests/unit/nest/places.controller.test.ts | 5 +- .../tests/unit/nest/places.directions.test.ts | 29 +++---- server/tests/unit/nest/places.service.test.ts | 57 +++++++------ .../tests/unit/nest/trips.controller.test.ts | 5 +- .../nest/unsplash/unsplash.service.test.ts | 4 +- .../tests/unit/nest/vacay.controller.test.ts | 11 +-- server/tests/unit/nest/vacay.service.test.ts | 68 ++++++++-------- 28 files changed, 227 insertions(+), 269 deletions(-) diff --git a/server/src/nest/collections/collections.controller.ts b/server/src/nest/collections/collections.controller.ts index 411307fc2..5a9a8f581 100644 --- a/server/src/nest/collections/collections.controller.ts +++ b/server/src/nest/collections/collections.controller.ts @@ -312,20 +312,14 @@ export class CollectionsController { if (!(await this.collections.isOwner(user.id, body.collection_id))) { throw new HttpException({ error: 'Only the owner can invite' }, 403); } - const result = await this.collections.sendInvite(body.collection_id, user.id, user.username, user.email, body.user_id, body.role); - if (result.error) { - throw new HttpException({ error: result.error }, result.status!); - } + await this.collections.sendInvite(body.collection_id, user.id, user.username, user.email, body.user_id, body.role); return { success: true }; } @Post('invite/accept') @HttpCode(200) async acceptInvite(@CurrentUser() user: User, @Body() body: CollectionInviteActionDto, @Headers('x-socket-id') socketId?: string) { - const result = await this.collections.acceptInvite(user.id, body.collection_id, socketId); - if (result.error) { - throw new HttpException({ error: result.error }, result.status!); - } + await this.collections.acceptInvite(user.id, body.collection_id, socketId); return { success: true }; } diff --git a/server/src/nest/collections/collections.mcp.ts b/server/src/nest/collections/collections.mcp.ts index b2d1a42d0..0b1035b18 100644 --- a/server/src/nest/collections/collections.mcp.ts +++ b/server/src/nest/collections/collections.mcp.ts @@ -363,8 +363,7 @@ export class CollectionsMcp { // of two where an unexpected throw escaped to the SDK instead of isError. try { const me = await this.users.findUsernameEmail(ctx.userId); - const res = await this.collections.sendInvite(collection_id, ctx.userId, me?.username ?? '', me?.email ?? '', user_id, role); - if (res.error) return { content: [{ type: 'text' as const, text: res.error }], isError: true }; + await this.collections.sendInvite(collection_id, ctx.userId, me?.username ?? '', me?.email ?? '', user_id, role); return ok({ success: true }); } catch (err) { return fail(err); } } @@ -429,8 +428,7 @@ export class CollectionsMcp { async acceptCollectionInvite({ collectionId }: { collectionId: number }, ctx: McpContext) { // try/catch added with the post-fold quirk pass (see inviteToCollection). try { - const res = await this.collections.acceptInvite(ctx.userId, collectionId, undefined); - if (res.error) return { content: [{ type: 'text' as const, text: res.error }], isError: true }; + await this.collections.acceptInvite(ctx.userId, collectionId, undefined); return ok({ success: true }); } catch (err) { return fail(err); } } diff --git a/server/src/nest/collections/collections.service.ts b/server/src/nest/collections/collections.service.ts index 86099f624..9defcfc5a 100644 --- a/server/src/nest/collections/collections.service.ts +++ b/server/src/nest/collections/collections.service.ts @@ -1,3 +1,4 @@ +import { DomainError } from '../common/domain-error'; import path from 'path'; import { Injectable } from '@nestjs/common'; import { InjectRepository } from '@mikro-orm/nestjs'; @@ -98,9 +99,7 @@ function serializeLinks(links: CollectionLink[] | undefined): string | null { // --------------------------------------------------------------------------- function httpError(status: number, message: string): never { - const err = new Error(message) as Error & { status: number }; - err.status = status; - throw err; + throw new DomainError(status, message); } /** Re-exported under this service's historical name — the underlying type @@ -1365,17 +1364,17 @@ export class CollectionsService { async sendInvite( collectionId: number, inviterId: number, inviterUsername: string, inviterEmail: string, targetUserId: number, role: 'viewer' | 'editor' | 'admin' = 'editor', - ): Promise<{ error?: string; status?: number }> { - if (!(await this.isOwner(inviterId, collectionId))) return { error: 'Not allowed', status: 403 }; - if (targetUserId === inviterId) return { error: 'Cannot invite yourself', status: 400 }; + ): Promise { + if (!(await this.isOwner(inviterId, collectionId))) throw new DomainError(403, 'Not allowed'); + if (targetUserId === inviterId) throw new DomainError(400, 'Cannot invite yourself'); const targetUser = await this.users.findIdUsername(targetUserId); - if (!targetUser) return { error: 'User not found', status: 404 }; + if (!targetUser) throw new DomainError(404, 'User not found'); const existing = await this.members.findByCollectionAndUser(collectionId, targetUserId); if (existing) { - if (existing.status === 'accepted') return { error: 'Already a member', status: 400 }; - if (existing.status === 'pending') return { error: 'Invite already pending', status: 400 }; + if (existing.status === 'accepted') throw new DomainError(400, 'Already a member'); + if (existing.status === 'pending') throw new DomainError(400, 'Invite already pending'); } await this.members.insertInvite(collectionId, targetUserId, role); @@ -1387,16 +1386,13 @@ export class CollectionsService { // reaches nothing in this direction — and it hid the edge while handing the // send a second NotificationsService built outside the container. this.notifications.send({ event: 'collection_invite', actorId: inviterId, scope: 'user', targetId: targetUserId, params: { actor: inviterEmail, collectionId: String(collectionId) } }).catch(() => {}); - - return {}; } - async acceptInvite(userId: number, collectionId: number, socketId: string | undefined): Promise<{ error?: string; status?: number }> { + async acceptInvite(userId: number, collectionId: number, socketId: string | undefined): Promise { const inviteId = await this.members.findPendingInvite(collectionId, userId); - if (inviteId === undefined) return { error: 'No pending invite', status: 404 }; + if (inviteId === undefined) throw new DomainError(404, 'No pending invite'); await this.members.accept(inviteId); await this.notifyCollectionUsers(collectionId, socketId, 'collections:accepted'); - return {}; } async declineInvite(userId: number, collectionId: number, socketId: string | undefined): Promise { diff --git a/server/src/nest/files/files-download.controller.ts b/server/src/nest/files/files-download.controller.ts index 9d17a683f..280da5c9b 100644 --- a/server/src/nest/files/files-download.controller.ts +++ b/server/src/nest/files/files-download.controller.ts @@ -32,9 +32,6 @@ export class FilesDownloadController { @Param('id') id: string, ): Promise { const auth = await this.files.authenticateDownload(req); - if ('error' in auth) { - throw new HttpException({ error: auth.error }, auth.status); - } const trip = await this.files.verifyTripAccess(tripId, auth.userId); if (!trip) { diff --git a/server/src/nest/files/files.service.ts b/server/src/nest/files/files.service.ts index 3303da289..7e2080725 100644 --- a/server/src/nest/files/files.service.ts +++ b/server/src/nest/files/files.service.ts @@ -1,3 +1,4 @@ +import { DomainError } from '../common/domain-error'; import { Injectable } from '@nestjs/common'; import { InjectRepository } from '@mikro-orm/nestjs'; import path from 'path'; @@ -156,7 +157,7 @@ export class FilesService { // download route) // --------------------------------------------------------------------------- - async authenticateDownload(req: Request): Promise<{ userId: number } | { error: string; status: number }> { + async authenticateDownload(req: Request): Promise<{ userId: number }> { const cookieToken = (req as { cookies?: Record }).cookies?.trek_session; const authHeader = req.headers['authorization']; const bearerToken = authHeader ? (authHeader.split(' ')[1] || undefined) : undefined; @@ -169,17 +170,17 @@ export class FilesService { // previously this bypassed the check and stolen download tokens stayed // valid across a password reset. const user = await verifyJwtAndLoadUser(jwtToken, this.em.getRepository(Users), this.em.getRepository(UserSessions)); - if (!user) return { error: 'Invalid or expired token', status: 401 }; + if (!user) throw new DomainError(401, 'Invalid or expired token'); return { userId: user.id }; } if (queryToken) { const uid = this.tokens.consume(queryToken, 'download'); - if (!uid) return { error: 'Invalid or expired token', status: 401 }; + if (!uid) throw new DomainError(401, 'Invalid or expired token'); return { userId: uid }; } - return { error: 'Authentication required', status: 401 }; + throw new DomainError(401, 'Authentication required'); } // --------------------------------------------------------------------------- diff --git a/server/src/nest/places/places.controller.ts b/server/src/nest/places/places.controller.ts index a03dd792a..0a2ead26e 100644 --- a/server/src/nest/places/places.controller.ts +++ b/server/src/nest/places/places.controller.ts @@ -291,9 +291,6 @@ export class PlacesController { : isDirectionsUrl(url) ? await this.places.importGoogleDirections(tripId, url, opts) : await this.places.importGoogleList(tripId, url, opts); - if ('error' in result) { - throw new HttpException({ error: result.error }, result.status); - } for (const place of result.places) { this.places.broadcast(tripId, 'place:created', { place }, socketId); } @@ -479,9 +476,6 @@ export class PlacesController { async image(@CurrentUser() user: User, @Param('tripId') tripId: string, @Param('id') id: string) { try { const result = await this.places.searchImage(tripId, id, user.id); - if ('error' in result) { - throw new HttpException({ error: result.error }, result.status); - } return { photos: result.photos }; } catch (err: unknown) { if (err instanceof HttpException) throw err; diff --git a/server/src/nest/places/places.mcp.ts b/server/src/nest/places/places.mcp.ts index 28dcb1eed..444e8c056 100644 --- a/server/src/nest/places/places.mcp.ts +++ b/server/src/nest/places/places.mcp.ts @@ -390,10 +390,6 @@ export class PlacesMcp { : await this.places.importGoogleList(String(tripId), url, opts)) : await this.places.importNaverList(String(tripId), url, opts); - if ('error' in result) { - return { content: [{ type: 'text' as const, text: result.error }], isError: true }; - } - for (const place of result.places) { this.guards.safeBroadcast(tripId, 'place:created', { place }); } diff --git a/server/src/nest/places/places.service.ts b/server/src/nest/places/places.service.ts index 621828738..67a07c60b 100644 --- a/server/src/nest/places/places.service.ts +++ b/server/src/nest/places/places.service.ts @@ -1,3 +1,4 @@ +import { DomainError } from '../common/domain-error'; import path from 'node:path'; import { Injectable } from '@nestjs/common'; import { InjectRepository } from '@mikro-orm/nestjs'; @@ -70,7 +71,6 @@ import { type GpxImportOptions, type GpxImportResult, type KmlImportOptions, - type ListImportError, type ListImportOptions, type ListImportResult, type PlaceImportResult, @@ -1144,9 +1144,9 @@ export class PlacesService { // Import Google Maps list // ------------------------------------------------------------------------- - async importGoogleList(tripId: string, url: string, opts?: ListImportOptions): Promise { + async importGoogleList(tripId: string, url: string, opts?: ListImportOptions): Promise { const read = await this.placeImport.readGoogleList(url); - if ('error' in read) return read; + if ('error' in read) throw new DomainError(read.status, read.error); // A short link that lands on a route is a directions link: imported as one, // with the resolved URL, so the hop is not made twice. if ('directions' in read) return this.importGoogleDirections(tripId, read.directions, opts); @@ -1246,9 +1246,9 @@ export class PlacesService { * The stops of a route somebody else planned, read and placed by * PlaceImportService (no API key, the stops are in the link), stored here. */ - async importGoogleDirections(tripId: string, url: string, opts?: ListImportOptions): Promise { + async importGoogleDirections(tripId: string, url: string, opts?: ListImportOptions): Promise { const read = await this.placeImport.readGoogleDirections(url, (query) => this.maps.geocodeQuery(query)); - if ('error' in read) return read; + if ('error' in read) throw new DomainError(read.status, read.error); const { places, unplaceable } = read; const { created, skipped } = await this.storeGooglePlaces(tripId, places); @@ -1267,9 +1267,9 @@ export class PlacesService { // Import Naver Maps list // ------------------------------------------------------------------------- - async importNaverList(tripId: string, url: string, opts?: ListImportOptions): Promise { + async importNaverList(tripId: string, url: string, opts?: ListImportOptions): Promise { const read = await this.placeImport.readNaverList(url); - if ('error' in read) return read; + if ('error' in read) throw new DomainError(read.status, read.error); const { listName, places } = read; // Rule 21 / M1 (Task 9 fix wave) — same ruling as `importGpxRows`: the @@ -1514,16 +1514,16 @@ export class PlacesService { // `toRowId` first (Task 3 review H1, absorbed here) — same gate shape // as every other place-id route in this service. const id = toRowId(placeId); - if (id === null) return { error: 'Place not found', status: 404 }; + if (id === null) throw new DomainError(404, 'Place not found'); // Rule 21 (H1): the trip id gets the same `toRowId` treatment as the // place id above, parsed once and used for the one downstream read — // `Number(tripId)` used to disagree with it and let a hex trip id read // a different trip's place. const tid = toRowId(tripId); - if (tid === null) return { error: 'Place not found', status: 404 }; + if (tid === null) throw new DomainError(404, 'Place not found'); // PL48 — same statement as PL9 (`applyUpdate`'s pre-image read). const place = await this.placesRepo.findInTrip(id, tid); - if (!place) return { error: 'Place not found', status: 404 }; + if (!place) throw new DomainError(404, 'Place not found'); return this.unsplash.searchUnsplashPhotos(place.name + (place.address ? ' ' + place.address : ''), 5, await this.unsplash.getUnsplashKey(userId)); } diff --git a/server/src/nest/trips/trips.controller.ts b/server/src/nest/trips/trips.controller.ts index 2fba2db01..71e90bd4c 100644 --- a/server/src/nest/trips/trips.controller.ts +++ b/server/src/nest/trips/trips.controller.ts @@ -93,9 +93,6 @@ export class TripsController { async coverImages(@CurrentUser() user: User, @Query('query') query?: string) { try { const result = await this.trips.searchCoverImages(query || '', user.id); - if ('error' in result) { - throw new HttpException({ error: result.error }, result.status); - } return { photos: result.photos }; } catch (err: unknown) { if (err instanceof HttpException) throw err; diff --git a/server/src/nest/trips/trips.mcp.ts b/server/src/nest/trips/trips.mcp.ts index c804911f6..df68262ad 100644 --- a/server/src/nest/trips/trips.mcp.ts +++ b/server/src/nest/trips/trips.mcp.ts @@ -5,6 +5,7 @@ import { errorResult, ok, } from '../../nest-mcp'; import { McpToolGuardsService } from '../mcp-shared/mcp-tool-guards.service'; +import { DomainError } from '../common/domain-error'; import { tripIdPromptArg } from '../mcp-shared/prompt-args'; import { z } from 'zod'; import { CalendarService } from '../calendar/calendar.service'; @@ -202,9 +203,11 @@ export class TripsMcp { async searchCoverImages({ query }: { query: string }, ctx: McpContext) { try { const result = await this.trips.searchCoverImages(query, ctx.userId); - if ('error' in result) return errorResult(result.error); return ok({ photos: result.photos }); } catch (err) { + // A refusal (no query, Unsplash answering an error) keeps its own text; + // the registry's error mapper turns it into the tool result. + if (err instanceof DomainError) throw err; console.error('Unsplash cover image error:', err); return errorResult('Error searching for cover images'); } diff --git a/server/src/nest/unsplash/unsplash.service.ts b/server/src/nest/unsplash/unsplash.service.ts index c48fe25bf..de6705992 100644 --- a/server/src/nest/unsplash/unsplash.service.ts +++ b/server/src/nest/unsplash/unsplash.service.ts @@ -1,3 +1,4 @@ +import { DomainError } from '../common/domain-error'; import { Readable } from 'node:stream'; import { v4 as uuidv4 } from 'uuid'; import { Injectable } from '@nestjs/common'; @@ -81,7 +82,7 @@ export class UnsplashService { async searchUnsplashPhotos(query: string, perPage = 9, accessKey?: string | null) { const trimmed = query.trim(); if (!trimmed) { - return { error: 'Search query is required', status: 400 }; + throw new DomainError(400, 'Search query is required'); } const params = new URLSearchParams({ @@ -116,11 +117,11 @@ export class UnsplashService { try { data = await response.json() as UnsplashSearchResponse; } catch { - return { error: 'Unsplash search unavailable', status: response.ok ? 502 : response.status }; + throw new DomainError(response.ok ? 502 : response.status, 'Unsplash search unavailable'); } if (!response.ok) { - return { error: data.errors?.[0] || data.error || 'Unsplash search unavailable', status: response.status }; + throw new DomainError(response.status, data.errors?.[0] || data.error || 'Unsplash search unavailable'); } const photos: UnsplashPhoto[] = (data.results || []) diff --git a/server/src/nest/vacay/vacay.controller.ts b/server/src/nest/vacay/vacay.controller.ts index 9e21fcd2c..5247eeaaf 100644 --- a/server/src/nest/vacay/vacay.controller.ts +++ b/server/src/nest/vacay/vacay.controller.ts @@ -124,20 +124,14 @@ export class VacayController { throw new HttpException({ error: 'user_id required' }, 400); } const plan = await this.vacay.getActivePlan(user.id); - const result = await this.vacay.sendInvite(plan.id, user.id, user.username, user.email, body.user_id as number); - if (result.error) { - throw new HttpException({ error: result.error }, result.status!); - } + await this.vacay.sendInvite(plan.id, user.id, user.username, user.email, body.user_id as number); return { success: true }; } @Post('invite/accept') @HttpCode(200) async acceptInvite(@CurrentUser() user: User, @Body() body: VacayInviteActionDto, @Headers('x-socket-id') socketId?: string) { - const result = await this.vacay.acceptInvite(user.id, body.plan_id as number, socketId); - if (result.error) { - throw new HttpException({ error: result.error }, result.status!); - } + await this.vacay.acceptInvite(user.id, body.plan_id as number, socketId); return { success: true }; } @@ -286,10 +280,7 @@ export class VacayController { if (!body.user_id) { throw new HttpException({ error: 'user_id required' }, 400); } - const result = await this.vacay.shareCalendar(user.id, user.email, Number.parseInt(String(body.user_id)), socketId); - if (result.error) { - throw new HttpException({ error: result.error }, result.status!); - } + await this.vacay.shareCalendar(user.id, user.email, Number.parseInt(String(body.user_id)), socketId); return { success: true }; } @@ -327,27 +318,18 @@ export class VacayController { @Get('holidays/countries') async holidayCountries() { const result = await this.vacay.getCountries(); - if (result.error) { - throw new HttpException({ error: result.error }, 502); - } return result.data; } @Get('holidays/:year/:country') async holidays(@Param('year') year: string, @Param('country') country: string) { const result = await this.vacay.getHolidays(year, country); - if (result.error) { - throw new HttpException({ error: result.error }, 502); - } return result.data; } @Get('school-holidays/regions/:country') async schoolHolidayRegions(@Param('country') country: string) { const result = await this.vacay.getSchoolHolidayRegions(country, country.toUpperCase() === 'DE' ? 'DE' : 'EN'); - if (result.error) { - throw new HttpException({ error: result.error }, 502); - } return result.data; } @@ -372,9 +354,6 @@ export class VacayController { private async schoolHolidays(year: string, country: string, subdivision?: string, group?: string) { const result = await this.vacay.getSchoolHolidays(year, country, subdivision, country.toUpperCase() === 'DE' ? 'DE' : 'EN', group); - if (result.error) { - throw new HttpException({ error: result.error }, 502); - } return result.data; } } diff --git a/server/src/nest/vacay/vacay.mcp.ts b/server/src/nest/vacay/vacay.mcp.ts index e11ac6e7e..266e677f2 100644 --- a/server/src/nest/vacay/vacay.mcp.ts +++ b/server/src/nest/vacay/vacay.mcp.ts @@ -5,6 +5,7 @@ import { TOOL_ANNOTATIONS_NON_IDEMPOTENT, errorResult, ok, } from '../../nest-mcp'; +import { catchDomainError, DomainError } from '../common/domain-error'; import { z } from 'zod'; import { vacayAddHolidayCalendarRequestSchema, @@ -185,8 +186,7 @@ export class VacayMcp { const planId = await this.vacay.getActivePlanId(ctx.userId); const me = await this.users.findUsernameEmail(ctx.userId); if (!me) return errorResult('User not found.'); - const result = await this.vacay.sendInvite(planId, ctx.userId, me.username, me.email, targetUserId); - if (result.error) return errorResult(result.error); + await this.vacay.sendInvite(planId, ctx.userId, me.username, me.email, targetUserId); return ok({ success: true }); } @@ -201,8 +201,7 @@ export class VacayMcp { access: { group: 'vacay', mode: 'write' }, }) async acceptVacayInvite({ planId }: { planId: number }, ctx: McpContext) { - const result = await this.vacay.acceptInvite(ctx.userId, planId, undefined); - if (result.error) return errorResult(result.error); + await this.vacay.acceptInvite(ctx.userId, planId, undefined); return ok({ success: true }); } @@ -468,7 +467,6 @@ export class VacayMcp { }) async listHolidayCountries(_args: Record, _ctx: McpContext) { const result = await this.vacay.getCountries(); - if (result.error) return errorResult(result.error); return ok({ countries: result.data }); } @@ -485,7 +483,6 @@ export class VacayMcp { }) async listHolidays({ country, year }: { country: string; year: number }, _ctx: McpContext) { const result = await this.vacay.getHolidays(String(year), country); - if (result.error) return errorResult(result.error); return ok({ holidays: result.data }); } @@ -501,7 +498,6 @@ export class VacayMcp { }) async listSchoolHolidayRegions({ country }: { country: string }, _ctx: McpContext) { const result = await this.vacay.getSchoolHolidayRegions(country, schoolHolidayLanguage(country)); - if (result.error) return errorResult(result.error); return ok({ regions: result.data, calendar_regions: calendarRegionCodes(country, result.data) }); } @@ -523,7 +519,6 @@ export class VacayMcp { _ctx: McpContext, ) { const result = await this.vacay.getSchoolHolidays(String(year), country, subdivision, schoolHolidayLanguage(country), group); - if (result.error) return errorResult(result.error); return ok({ holidays: result.data }); } @@ -564,8 +559,7 @@ export class VacayMcp { async shareVacayCalendar({ targetUserId }: { targetUserId: number }, ctx: McpContext) { const me = await this.users.findUsernameEmail(ctx.userId); if (!me) return errorResult('User not found.'); - const result = await this.vacay.shareCalendar(ctx.userId, me.email, targetUserId); - if (result.error) return errorResult(result.error); + await this.vacay.shareCalendar(ctx.userId, me.email, targetUserId); return ok({ success: true }); } @@ -662,7 +656,8 @@ export class VacayMcp { }); if (!plan.holidays_enabled || !plan.holidays_region) return json([]); const yearStr = Array.isArray(year) ? year[0] : year; - const result = await this.vacay.getHolidays(yearStr, plan.holidays_region); - return json(result.data ?? []); + // A failed fetch reads as no holidays here, as it always has. + const result = await catchDomainError(() => this.vacay.getHolidays(yearStr, plan.holidays_region)); + return json(result instanceof DomainError ? [] : (result.data ?? [])); } } diff --git a/server/src/nest/vacay/vacay.service.ts b/server/src/nest/vacay/vacay.service.ts index fc5b64cf3..89b13e12c 100644 --- a/server/src/nest/vacay/vacay.service.ts +++ b/server/src/nest/vacay/vacay.service.ts @@ -1,3 +1,4 @@ +import { DomainError } from '../common/domain-error'; import { Injectable, BadRequestException } from '@nestjs/common'; import { InjectRepository } from '@mikro-orm/nestjs'; import { manualSchoolRegionId } from '@trek/shared'; @@ -699,22 +700,22 @@ export class VacayService { // Invitations // ------------------------------------------------------------------------- - async sendInvite(planId: number, inviterId: number, inviterUsername: string, inviterEmail: string, targetUserId: number): Promise<{ error?: string; status?: number }> { - if (targetUserId === inviterId) return { error: 'Cannot invite yourself', status: 400 }; + async sendInvite(planId: number, inviterId: number, inviterUsername: string, inviterEmail: string, targetUserId: number): Promise { + if (targetUserId === inviterId) throw new DomainError(400, 'Cannot invite yourself'); // The picker no longer offers guests, but the id arrives from the client, so the // write path has to refuse them too rather than trust the list it handed out. const targetUser = await this.members.findInvitableUser(targetUserId); - if (!targetUser) return { error: 'User not found', status: 404 }; + if (!targetUser) throw new DomainError(404, 'User not found'); const existing = await this.members.findMembership(planId, targetUserId); if (existing) { - if (existing.status === 'accepted') return { error: 'Already fused', status: 400 }; - if (existing.status === 'pending') return { error: 'Invite already pending', status: 400 }; + if (existing.status === 'accepted') throw new DomainError(400, 'Already fused'); + if (existing.status === 'pending') throw new DomainError(400, 'Invite already pending'); } const targetFusion = await this.members.findAcceptedForUser(targetUserId); - if (targetFusion) return { error: 'User is already fused with another plan', status: 400 }; + if (targetFusion) throw new DomainError(400, 'User is already fused with another plan'); await this.members.insertPending(planId, targetUserId); @@ -732,16 +733,14 @@ export class VacayService { // reaches nothing in this direction — and it hid the edge while handing the // send a second NotificationsService built outside the container. this.notifications.send({ event: 'vacay_invite', actorId: inviterId, scope: 'user', targetId: targetUserId, params: { actor: inviterEmail, planId: String(planId) } }).catch(() => {}); - - return {}; } - async acceptInvite(userId: number, planId: number, socketId: string | undefined): Promise<{ error?: string; status?: number }> { + async acceptInvite(userId: number, planId: number, socketId: string | undefined): Promise { // The accept flow is a multi-statement write (status flip + entry/year/color // migration + seeding) — atomic, so a failure can't leave the member half-fused. - const result = await this.uow.transactional(async (): Promise<{ error?: string; status?: number }> => { + await this.uow.transactional(async () => { const invite = await this.members.findPending(planId, userId); - if (!invite) return { error: 'No pending invite', status: 404 }; + if (!invite) throw new DomainError(404, 'No pending invite'); await this.members.accept(invite.id); @@ -781,13 +780,11 @@ export class VacayService { for (const y of targetYears) { await this.userYears.insertIgnore(userId, planId, y, 30, 0); } - return {}; }); - // Only announce a fusion that actually happened — the transaction returns the - // refusal for an invite that was already gone. - if (!result.error) await this.notifyPlanUsers(planId, socketId, 'vacay:accepted'); - return result; + // Only announce a fusion that actually happened: an invite that was already + // gone refuses inside the transaction and never reaches this line. + await this.notifyPlanUsers(planId, socketId, 'vacay:accepted'); } async declineInvite(userId: number, planId: number, socketId: string | undefined): Promise { @@ -928,18 +925,18 @@ export class VacayService { return { outgoing, incoming }; } - async shareCalendar(ownerId: number, ownerEmail: string, targetUserId: number, socketId?: string): Promise<{ error?: string; status?: number }> { - if (targetUserId === ownerId) return { error: 'Cannot share with yourself', status: 400 }; + async shareCalendar(ownerId: number, ownerEmail: string, targetUserId: number, socketId?: string): Promise { + if (targetUserId === ownerId) throw new DomainError(400, 'Cannot share with yourself'); const targetOk = await this.shares.existsInvitableUser(targetUserId); - if (!targetOk) return { error: 'User not found', status: 404 }; + if (!targetOk) throw new DomainError(404, 'User not found'); const existing = await this.shares.findByOwnerAndUser(ownerId, targetUserId); - if (existing) return { error: 'Already shared', status: 400 }; + if (existing) throw new DomainError(400, 'Already shared'); // Plan members already see the whole calendar — sharing with them is moot. if ((await this.getPlanUsers(await this.getActivePlanId(ownerId))).find(u => u.id === targetUserId)) { - return { error: 'User is already in your calendar', status: 400 }; + throw new DomainError(400, 'User is already in your calendar'); } await this.shares.insertShare(ownerId, targetUserId); @@ -951,8 +948,6 @@ export class VacayService { } catch { /* websocket not available */ } this.notifications.send({ event: 'vacay_share', actorId: ownerId, scope: 'user', targetId: targetUserId, params: { actor: ownerEmail } }).catch(() => {}); - - return {}; } /** @@ -1315,43 +1310,43 @@ export class VacayService { // Holidays (nager.at proxy with cache) // ------------------------------------------------------------------------- - async getCountries(): Promise<{ data?: unknown; error?: string }> { + async getCountries(): Promise<{ data?: unknown }> { const cacheKey = 'countries'; const cached = this.holidayCache.get(cacheKey); if (cached && Date.now() - cached.time < CACHE_TTL) return { data: cached.data }; try { const resp = await fetch('https://date.nager.at/api/v3/AvailableCountries', { signal: AbortSignal.timeout(FETCH_TIMEOUT_MS) }); - if (!resp.ok) { discardBody(resp); return { error: 'Failed to fetch countries' }; } + if (!resp.ok) { discardBody(resp); throw new DomainError(502, 'Failed to fetch countries'); } const data = await readCappedJson(resp, MAX_HOLIDAY_BYTES); - if (data === undefined) return { error: 'Failed to fetch countries' }; + if (data === undefined) throw new DomainError(502, 'Failed to fetch countries'); this.holidayCache.set(cacheKey, { data, time: Date.now() }); return { data }; } catch { - return { error: 'Failed to fetch countries' }; + throw new DomainError(502, 'Failed to fetch countries'); } } - async getHolidays(year: string, country: string): Promise<{ data?: unknown; error?: string }> { + async getHolidays(year: string, country: string): Promise<{ data?: unknown }> { // Both segments land in the URL path, so they are checked before the cache // lookup rather than after it. - if (!YEAR_RE.test(year) || !COUNTRY_RE.test(country)) return { error: 'Failed to fetch holidays' }; + if (!YEAR_RE.test(year) || !COUNTRY_RE.test(country)) throw new DomainError(502, 'Failed to fetch holidays'); const cacheKey = `${year}-${country}`; const cached = this.holidayCache.get(cacheKey); if (cached && Date.now() - cached.time < CACHE_TTL) return { data: cached.data }; try { const resp = await fetch(`https://date.nager.at/api/v3/PublicHolidays/${year}/${country}`, { signal: AbortSignal.timeout(FETCH_TIMEOUT_MS) }); - if (!resp.ok) { discardBody(resp); return { error: 'Failed to fetch holidays' }; } + if (!resp.ok) { discardBody(resp); throw new DomainError(502, 'Failed to fetch holidays'); } const data = await readCappedJson(resp, MAX_HOLIDAY_BYTES); - if (data === undefined) return { error: 'Failed to fetch holidays' }; + if (data === undefined) throw new DomainError(502, 'Failed to fetch holidays'); this.holidayCache.set(cacheKey, { data, time: Date.now() }); return { data }; } catch { - return { error: 'Failed to fetch holidays' }; + throw new DomainError(502, 'Failed to fetch holidays'); } } - async getSchoolHolidayRegions(country: string, language = 'EN'): Promise<{ data?: unknown; error?: string }> { - if (!COUNTRY_RE.test(country)) return { error: 'Failed to fetch school holiday regions' }; + async getSchoolHolidayRegions(country: string, language = 'EN'): Promise<{ data?: unknown }> { + if (!COUNTRY_RE.test(country)) throw new DomainError(502, 'Failed to fetch school holiday regions'); const normalizedLanguage = String(language || 'EN').slice(0, 2).toUpperCase(); const cacheKey = `school-regions-${country}-${normalizedLanguage}`; const cached = this.holidayCache.get(cacheKey); @@ -1364,21 +1359,21 @@ export class VacayService { if (!groupsResp.ok || !subdivisionsResp.ok) { discardBody(groupsResp); discardBody(subdivisionsResp); - return { error: 'Failed to fetch school holiday regions' }; + throw new DomainError(502, 'Failed to fetch school holiday regions'); } const groups = await readCappedJson(groupsResp, MAX_HOLIDAY_BYTES); const subdivisions = await readCappedJson(subdivisionsResp, MAX_HOLIDAY_BYTES); - if (groups === undefined || subdivisions === undefined) return { error: 'Failed to fetch school holiday regions' }; + if (groups === undefined || subdivisions === undefined) throw new DomainError(502, 'Failed to fetch school holiday regions'); const data = { groups, subdivisions }; this.holidayCache.set(cacheKey, { data, time: Date.now() }); return { data }; } catch { - return { error: 'Failed to fetch school holiday regions' }; + throw new DomainError(502, 'Failed to fetch school holiday regions'); } } - async getSchoolHolidays(year: string, country: string, subdivision?: string | null, language = 'EN', group?: string | null): Promise<{ data?: unknown; error?: string }> { - if (!YEAR_RE.test(year) || !COUNTRY_RE.test(country)) return { error: 'Failed to fetch school holidays' }; + async getSchoolHolidays(year: string, country: string, subdivision?: string | null, language = 'EN', group?: string | null): Promise<{ data?: unknown }> { + if (!YEAR_RE.test(year) || !COUNTRY_RE.test(country)) throw new DomainError(502, 'Failed to fetch school holidays'); const normalizedLanguage = String(language || 'EN').slice(0, 2).toUpperCase(); const normalizedSubdivision = subdivision || ''; const normalizedGroup = group || ''; @@ -1398,13 +1393,13 @@ export class VacayService { headers: { accept: 'text/json' }, signal: AbortSignal.timeout(FETCH_TIMEOUT_MS), }); - if (!resp.ok) { discardBody(resp); return { error: 'Failed to fetch school holidays' }; } + if (!resp.ok) { discardBody(resp); throw new DomainError(502, 'Failed to fetch school holidays'); } const data = await readCappedJson(resp, MAX_HOLIDAY_BYTES); - if (data === undefined) return { error: 'Failed to fetch school holidays' }; + if (data === undefined) throw new DomainError(502, 'Failed to fetch school holidays'); this.holidayCache.set(cacheKey, { data, time: Date.now() }); return { data }; } catch { - return { error: 'Failed to fetch school holidays' }; + throw new DomainError(502, 'Failed to fetch school holidays'); } } } diff --git a/server/tests/helpers/domain-error.ts b/server/tests/helpers/domain-error.ts index 5da09a72a..ffac80012 100644 --- a/server/tests/helpers/domain-error.ts +++ b/server/tests/helpers/domain-error.ts @@ -7,7 +7,8 @@ import { DomainError } from '../../src/nest/common/domain-error'; */ export async function asLegacyResult(call: Promise | T): Promise { try { - return (await call) as T & { error?: string; status?: number }; + // A call that resolves to nothing (a void write) reads as the empty success it used to return. + return ((await call) ?? {}) as T & { error?: string; status?: number }; } catch (err) { if (err instanceof DomainError) { return { ...err.details, error: err.publicMessage, status: err.getStatus() } as T & { diff --git a/server/tests/unit/mcp/tools-places.test.ts b/server/tests/unit/mcp/tools-places.test.ts index e25489988..f4fc46a54 100644 --- a/server/tests/unit/mcp/tools-places.test.ts +++ b/server/tests/unit/mcp/tools-places.test.ts @@ -7,6 +7,7 @@ * registry in tests/helpers/mcp-test-controllers.ts, so the assertions below * exercise the @Tool/@ResourceTemplate path instead of the deleted registrar. */ +import { DomainError } from '../../../src/nest/common/domain-error'; import { describe, it, expect, vi, beforeAll, beforeEach, afterAll } from 'vitest'; import { db as testDb } from '../../../src/db/database'; @@ -1012,10 +1013,10 @@ describe('Tool: import_places_from_url', () => { list.mockRestore(); }); - it('surfaces the naver importer\'s { error, status } as a tool error', async () => { + it('surfaces the naver importer\'s refusal as a tool error', async () => { const { user } = createUser(testDb); const trip = createTrip(testDb, user.id); - const spy = vi.spyOn(PlacesService.prototype, 'importNaverList').mockResolvedValue({ error: 'List is empty or could not be read', status: 400 }); + const spy = vi.spyOn(PlacesService.prototype, 'importNaverList').mockRejectedValue(new DomainError(400, 'List is empty or could not be read')); await withHarness(user.id, async (h) => { const result = await h.client.callTool({ diff --git a/server/tests/unit/mcp/tools-vacay.test.ts b/server/tests/unit/mcp/tools-vacay.test.ts index 08959173a..038cd5016 100644 --- a/server/tests/unit/mcp/tools-vacay.test.ts +++ b/server/tests/unit/mcp/tools-vacay.test.ts @@ -13,6 +13,7 @@ * trek://vacay/holidays/{year} — these ride the registry too (attached inside * registerTools), so `withTools` must stay on even for resource reads. */ +import { DomainError } from '../../../src/nest/common/domain-error'; import { describe, it, expect, vi, beforeAll, beforeEach, afterEach, afterAll } from 'vitest'; import { db as testDb } from '../../../src/db/database'; @@ -720,7 +721,7 @@ describe('Tool: list_school_holiday_regions', () => { it('surfaces the upstream failure', async () => { const { user } = createUser(testDb); - schoolRegionsSpy.mockResolvedValueOnce({ error: 'Failed to fetch school holiday regions' }); + schoolRegionsSpy.mockRejectedValueOnce(new DomainError(502, 'Failed to fetch school holiday regions')); await withHarness(user.id, async (h) => { const result = await h.client.callTool({ name: 'list_school_holiday_regions', arguments: { country: 'DE' } }); expect(result.isError).toBe(true); @@ -757,7 +758,7 @@ describe('Tool: list_school_holidays', () => { it('surfaces the upstream failure', async () => { const { user } = createUser(testDb); - schoolHolidaysSpy.mockResolvedValueOnce({ error: 'Failed to fetch school holidays' }); + schoolHolidaysSpy.mockRejectedValueOnce(new DomainError(502, 'Failed to fetch school holidays')); await withHarness(user.id, async (h) => { const result = await h.client.callTool({ name: 'list_school_holidays', arguments: { country: 'DE', year: 2025 } }); expect(result.isError).toBe(true); diff --git a/server/tests/unit/nest/collections.controller.test.ts b/server/tests/unit/nest/collections.controller.test.ts index ab3a9ff12..def743c53 100644 --- a/server/tests/unit/nest/collections.controller.test.ts +++ b/server/tests/unit/nest/collections.controller.test.ts @@ -1,3 +1,4 @@ +import { DomainError } from '../../../src/nest/common/domain-error'; import { describe, it, expect, vi, afterEach } from 'vitest'; import { RuntimeEnvService } from '../../../src/nest/app-config/runtime-env.service'; import { HttpException } from '@nestjs/common'; @@ -140,7 +141,7 @@ describe('CollectionsController', () => { .toEqual({ status: 403, body: { error: 'Only the owner can invite' } }); }); it('surfaces a sendInvite error with its status', async () => { - const svc = makeService({ sendInvite: vi.fn().mockResolvedValue({ error: 'Already a member', status: 409 }) }); + const svc = makeService({ sendInvite: vi.fn().mockRejectedValue(new DomainError(409, 'Already a member')) }); expect(await thrownAsync(() => new CollectionsController(svc, new RuntimeEnvService(), storageStub).invite(user, { collection_id: 3, user_id: 2 } as never))) .toEqual({ status: 409, body: { error: 'Already a member' } }); }); @@ -149,7 +150,7 @@ describe('CollectionsController', () => { expect(await new CollectionsController(svc, new RuntimeEnvService(), storageStub).invite(user, { collection_id: 3, user_id: 2 } as never)).toEqual({ success: true }); }); it('accept surfaces an error, else succeeds', async () => { - const bad = makeService({ acceptInvite: vi.fn().mockResolvedValue({ error: 'Gone', status: 404 }) }); + const bad = makeService({ acceptInvite: vi.fn().mockRejectedValue(new DomainError(404, 'Gone')) }); expect(await thrownAsync(() => new CollectionsController(bad, new RuntimeEnvService(), storageStub).acceptInvite(user, { collection_id: 3 } as never, 'sid'))) .toEqual({ status: 404, body: { error: 'Gone' } }); const svc = makeService(); diff --git a/server/tests/unit/nest/collections.service.test.ts b/server/tests/unit/nest/collections.service.test.ts index 641008513..28c60725e 100644 --- a/server/tests/unit/nest/collections.service.test.ts +++ b/server/tests/unit/nest/collections.service.test.ts @@ -9,6 +9,7 @@ * faithfully. Keeps its own clearCollections() reset (the shared * resetTestDb RESET_TABLES list has no collection tables). */ +import { asLegacyResult } from '../../helpers/domain-error'; import { describe, it, expect, vi, beforeAll, beforeEach, afterAll } from 'vitest'; vi.mock('../../../src/db/database', async () => { @@ -572,11 +573,11 @@ describe('fusion invitations', () => { it('COLLECTIONS-SVC-030: sendInvite — self 400, unknown 404, non-owner 403, happy path', async () => { const { owner, target, col } = await setup(); - expect((await svc.sendInvite(col.id, owner.id, owner.username, owner.email, owner.id)).status).toBe(400); - expect((await svc.sendInvite(col.id, owner.id, owner.username, owner.email, 99999)).status).toBe(404); - expect((await svc.sendInvite(col.id, target.id, target.username, target.email, owner.id)).status).toBe(403); // non-owner inviter + expect((await asLegacyResult(svc.sendInvite(col.id, owner.id, owner.username, owner.email, owner.id))).status).toBe(400); + expect((await asLegacyResult(svc.sendInvite(col.id, owner.id, owner.username, owner.email, 99999))).status).toBe(404); + expect((await asLegacyResult(svc.sendInvite(col.id, target.id, target.username, target.email, owner.id))).status).toBe(403); // non-owner inviter - const ok = await svc.sendInvite(col.id, owner.id, owner.username, owner.email, target.id); + const ok = await asLegacyResult(svc.sendInvite(col.id, owner.id, owner.username, owner.email, target.id)); expect(ok.error).toBeUndefined(); expect(broadcastToUser).toHaveBeenCalledWith(target.id, expect.objectContaining({ type: 'collections:invite' })); // the notification send is fire-and-forget via a dynamic import — flush microtasks. @@ -585,37 +586,37 @@ describe('fusion invitations', () => { it('COLLECTIONS-SVC-031: double-invite while pending → 400; existing member → 400', async () => { const { owner, target, col } = await setup(); - await svc.sendInvite(col.id, owner.id, owner.username, owner.email, target.id); - expect((await svc.sendInvite(col.id, owner.id, owner.username, owner.email, target.id)).status).toBe(400); - await svc.acceptInvite(target.id, col.id, undefined); - expect((await svc.sendInvite(col.id, owner.id, owner.username, owner.email, target.id)).error).toBe('Already a member'); + await asLegacyResult(svc.sendInvite(col.id, owner.id, owner.username, owner.email, target.id)); + expect((await asLegacyResult(svc.sendInvite(col.id, owner.id, owner.username, owner.email, target.id))).status).toBe(400); + await asLegacyResult(svc.acceptInvite(target.id, col.id, undefined)); + expect((await asLegacyResult(svc.sendInvite(col.id, owner.id, owner.username, owner.email, target.id))).error).toBe('Already a member'); }); it('COLLECTIONS-SVC-032: acceptInvite — 404 with no pending; flips to accepted → member now sees list', async () => { const { owner, target, col } = await setup(); - expect((await svc.acceptInvite(target.id, col.id, undefined)).status).toBe(404); - await svc.sendInvite(col.id, owner.id, owner.username, owner.email, target.id); - expect((await svc.acceptInvite(target.id, col.id, undefined)).error).toBeUndefined(); + expect((await asLegacyResult(svc.acceptInvite(target.id, col.id, undefined))).status).toBe(404); + await asLegacyResult(svc.sendInvite(col.id, owner.id, owner.username, owner.email, target.id)); + expect((await asLegacyResult(svc.acceptInvite(target.id, col.id, undefined))).error).toBeUndefined(); expect((await svc.listCollections(target.id)).collections.map((c) => c.id)).toContain(col.id); }); it('COLLECTIONS-SVC-033: accept-after-cancel → 404 (no orphan accept)', async () => { const { owner, target, col } = await setup(); - await svc.sendInvite(col.id, owner.id, owner.username, owner.email, target.id); + await asLegacyResult(svc.sendInvite(col.id, owner.id, owner.username, owner.email, target.id)); await svc.cancelInvite(col.id, owner.id, target.id); - expect((await svc.acceptInvite(target.id, col.id, undefined)).status).toBe(404); + expect((await asLegacyResult(svc.acceptInvite(target.id, col.id, undefined))).status).toBe(404); }); it('COLLECTIONS-SVC-034: declineInvite removes the pending row', async () => { const { owner, target, col } = await setup(); - await svc.sendInvite(col.id, owner.id, owner.username, owner.email, target.id); + await asLegacyResult(svc.sendInvite(col.id, owner.id, owner.username, owner.email, target.id)); await svc.declineInvite(target.id, col.id, undefined); expect(await membersOf(col.id)).toEqual({ n: 0 }); }); it('COLLECTIONS-SVC-035: cancelInvite is owner-only', async () => { const { owner, target, col } = await setup(); - await svc.sendInvite(col.id, owner.id, owner.username, owner.email, target.id); + await asLegacyResult(svc.sendInvite(col.id, owner.id, owner.username, owner.email, target.id)); await expect(svc.cancelInvite(col.id, target.id, target.id)).rejects.toThrow(); // non-owner await svc.cancelInvite(col.id, owner.id, target.id); // owner ok expect(await membersOf(col.id)).toEqual({ n: 0 }); @@ -623,8 +624,8 @@ describe('fusion invitations', () => { it('COLLECTIONS-SVC-036: leaveCollection — member ok, owner blocked (400)', async () => { const { owner, target, col } = await setup(); - await svc.sendInvite(col.id, owner.id, owner.username, owner.email, target.id); - await svc.acceptInvite(target.id, col.id, undefined); + await asLegacyResult(svc.sendInvite(col.id, owner.id, owner.username, owner.email, target.id)); + await asLegacyResult(svc.acceptInvite(target.id, col.id, undefined)); await svc.leaveCollection(target.id, col.id, undefined); expect((await svc.listCollections(target.id)).collections.map((c) => c.id)).not.toContain(col.id); @@ -638,8 +639,8 @@ describe('fusion invitations', () => { const colA = await svc.createCollection(owner.id, { name: 'A' }); const colB = await svc.createCollection(owner.id, { name: 'B' }); // target is accepted in A; must still be invitable to B - await svc.sendInvite(colA.id, owner.id, owner.username, owner.email, target.id); - await svc.acceptInvite(target.id, colA.id, undefined); + await asLegacyResult(svc.sendInvite(colA.id, owner.id, owner.username, owner.email, target.id)); + await asLegacyResult(svc.acceptInvite(target.id, colA.id, undefined)); const forB = (await svc.availableUsers(owner.id, colB.id)).map((u) => u.id); expect(forB).toContain(target.id); @@ -661,9 +662,9 @@ describe('fusion invitations', () => { it('COLLECTIONS-SVC-039: visibility = owner OR accepted member (pending does NOT grant access)', async () => { const { owner, target, col } = await setup(); - await svc.sendInvite(col.id, owner.id, owner.username, owner.email, target.id); + await asLegacyResult(svc.sendInvite(col.id, owner.id, owner.username, owner.email, target.id)); await expect(svc.getCollection(target.id, col.id)).rejects.toThrow(); // pending, no access yet - await svc.acceptInvite(target.id, col.id, undefined); + await asLegacyResult(svc.acceptInvite(target.id, col.id, undefined)); expect((await svc.getCollection(target.id, col.id)).collection.id).toBe(col.id); }); }); @@ -677,9 +678,9 @@ describe('deleteCollection', () => { const pending = createUser(testDb).user; const col = await svc.createCollection(owner.id, { name: 'Doomed' }); await svc.savePlace(owner.id, { collection_id: col.id, name: 'P' }); - await svc.sendInvite(col.id, owner.id, owner.username, owner.email, accepted.id); - await svc.acceptInvite(accepted.id, col.id, undefined); - await svc.sendInvite(col.id, owner.id, owner.username, owner.email, pending.id); + await asLegacyResult(svc.sendInvite(col.id, owner.id, owner.username, owner.email, accepted.id)); + await asLegacyResult(svc.acceptInvite(accepted.id, col.id, undefined)); + await asLegacyResult(svc.sendInvite(col.id, owner.id, owner.username, owner.email, pending.id)); // a non-owner member cannot delete await expect(svc.deleteCollection(accepted.id, col.id)).rejects.toThrow(); @@ -1028,9 +1029,9 @@ describe('membership lookups', () => { expect(await svc.findMembershipForUser(owner.id, col.id)).toEqual({ is_member: true, is_owner: true, status: 'accepted' }); expect(await svc.findMembershipForUser(outsider.id, col.id)).toEqual({ is_member: false, is_owner: false, status: null }); - await svc.sendInvite(col.id, owner.id, owner.username, owner.email, member.id); + await asLegacyResult(svc.sendInvite(col.id, owner.id, owner.username, owner.email, member.id)); expect(await svc.findMembershipForUser(member.id, col.id)).toEqual({ is_member: false, is_owner: false, status: 'pending' }); - await svc.acceptInvite(member.id, col.id, undefined); + await asLegacyResult(svc.acceptInvite(member.id, col.id, undefined)); expect(await svc.findMembershipForUser(member.id, col.id)).toEqual({ is_member: true, is_owner: false, status: 'accepted' }); }); }); @@ -1688,7 +1689,7 @@ describe('Plan 3h Task 1 — repository conversion parity', () => { await addMember(col.id, admin.id, 'admin'); await addMember(col.id, editor.id, 'editor'); await addMember(col.id, viewer.id, 'viewer'); - await svc.sendInvite(col.id, owner.id, owner.username, owner.email, pending.id); + await asLegacyResult(svc.sendInvite(col.id, owner.id, owner.username, owner.email, pending.id)); const tag = createTag(testDb, owner.id, { name: 'Foodie' }); const place = (await svc.savePlace(owner.id, { @@ -1748,9 +1749,9 @@ describe('Plan 3h Task 1 — repository conversion parity', () => { const accepted = createUser(testDb).user; const pending = createUser(testDb).user; const col = await svc.createCollection(owner.id, { name: 'Ordering proof' }); - await svc.sendInvite(col.id, owner.id, owner.username, owner.email, accepted.id); - await svc.acceptInvite(accepted.id, col.id, undefined); - await svc.sendInvite(col.id, owner.id, owner.username, owner.email, pending.id); + await asLegacyResult(svc.sendInvite(col.id, owner.id, owner.username, owner.email, accepted.id)); + await asLegacyResult(svc.acceptInvite(accepted.id, col.id, undefined)); + await asLegacyResult(svc.sendInvite(col.id, owner.id, owner.username, owner.email, pending.id)); broadcastToUser.mockClear(); // Captured at CALL time, not after `deleteCollection` returns — if the diff --git a/server/tests/unit/nest/files.controller.test.ts b/server/tests/unit/nest/files.controller.test.ts index ba320ada2..016107f85 100644 --- a/server/tests/unit/nest/files.controller.test.ts +++ b/server/tests/unit/nest/files.controller.test.ts @@ -1,3 +1,4 @@ +import { DomainError } from '../../../src/nest/common/domain-error'; import { describe, it, expect, vi, beforeEach, afterEach } from 'vitest'; import { RuntimeEnvService } from '../../../src/nest/app-config/runtime-env.service'; import { HttpException } from '@nestjs/common'; @@ -261,7 +262,7 @@ describe('FilesDownloadController', () => { const res = {} as Response; it('maps the auth error from authenticateDownload', async () => { - const s = dsvc({ authenticateDownload: vi.fn().mockReturnValue({ error: 'Authentication required', status: 401 }) }); + const s = dsvc({ authenticateDownload: vi.fn().mockRejectedValue(new DomainError(401, 'Authentication required')) }); expect(await rejected(new FilesDownloadController(s, dstor()).download(req, res, '5', '9'))).toEqual({ status: 401, body: { error: 'Authentication required' } }); }); diff --git a/server/tests/unit/nest/files.service.test.ts b/server/tests/unit/nest/files.service.test.ts index 055b65642..4bc8caf4b 100644 --- a/server/tests/unit/nest/files.service.test.ts +++ b/server/tests/unit/nest/files.service.test.ts @@ -6,6 +6,7 @@ * and the download-token auth over a real in-memory SQLite DB, plus the * files.bridge delegation (inside the src/nest coverage gate). */ +import { asLegacyResult } from '../../helpers/domain-error'; import { describe, it, expect, vi, beforeAll, beforeEach, afterAll, afterEach } from 'vitest'; import path from 'path'; @@ -808,7 +809,7 @@ describe('read-model parity: raw SQL vs. the repository-backed reads', () => { describe('authenticateDownload', () => { it('FILE-SVC-032: a valid session cookie wins over a bearer token', async () => { verifyJwtAndLoadUser.mockReturnValue({ id: 42 }); - const result = await svc.authenticateDownload(req({ cookie: 'cookie-jwt', bearer: 'bearer-jwt' })); + const result = await asLegacyResult(svc.authenticateDownload(req({ cookie: 'cookie-jwt', bearer: 'bearer-jwt' }))); expect(result).toEqual({ userId: 42 }); // The concrete expected argument, not expect.anything(): emStub.getRepository // always returns a fresh `{}` (task-1-review.md F6) — deep-equal, not a @@ -825,26 +826,26 @@ describe('authenticateDownload', () => { it('FILE-SVC-033: a bearer token is used when no cookie is present; invalid JWTs 401', async () => { verifyJwtAndLoadUser.mockReturnValue({ id: 7 }); - expect(await svc.authenticateDownload(req({ bearer: 'bearer-jwt' }))).toEqual({ userId: 7 }); + expect(await asLegacyResult(svc.authenticateDownload(req({ bearer: 'bearer-jwt' })))).toEqual({ userId: 7 }); expect(verifyJwtAndLoadUser).toHaveBeenCalledWith('bearer-jwt', {}, {}); // see FILE-SVC-032's comment expect(getRepository).toHaveBeenCalledWith(Users); expect(getRepository).toHaveBeenCalledWith(UserSessions); verifyJwtAndLoadUser.mockReturnValue(null); - expect(await svc.authenticateDownload(req({ bearer: 'stale' }))).toEqual({ error: 'Invalid or expired token', status: 401 }); + expect(await asLegacyResult(svc.authenticateDownload(req({ bearer: 'stale' })))).toEqual({ error: 'Invalid or expired token', status: 401 }); }); it('FILE-SVC-034: a ?token= ephemeral token is consumed with the download purpose', async () => { consumeEphemeralToken.mockReturnValue(9); - expect(await svc.authenticateDownload(req({ token: 'eph' }))).toEqual({ userId: 9 }); + expect(await asLegacyResult(svc.authenticateDownload(req({ token: 'eph' })))).toEqual({ userId: 9 }); expect(consumeEphemeralToken).toHaveBeenCalledWith('eph', 'download'); consumeEphemeralToken.mockReturnValue(null); - expect(await svc.authenticateDownload(req({ token: 'spent' }))).toEqual({ error: 'Invalid or expired token', status: 401 }); + expect(await asLegacyResult(svc.authenticateDownload(req({ token: 'spent' })))).toEqual({ error: 'Invalid or expired token', status: 401 }); }); it('FILE-SVC-035: no credentials at all is a 401 Authentication required', async () => { - expect(await svc.authenticateDownload(req({}))).toEqual({ error: 'Authentication required', status: 401 }); + expect(await asLegacyResult(svc.authenticateDownload(req({})))).toEqual({ error: 'Authentication required', status: 401 }); expect(verifyJwtAndLoadUser).not.toHaveBeenCalled(); expect(consumeEphemeralToken).not.toHaveBeenCalled(); }); diff --git a/server/tests/unit/nest/places.controller.test.ts b/server/tests/unit/nest/places.controller.test.ts index dac2cea9e..0f4feaf90 100644 --- a/server/tests/unit/nest/places.controller.test.ts +++ b/server/tests/unit/nest/places.controller.test.ts @@ -1,3 +1,4 @@ +import { DomainError } from '../../../src/nest/common/domain-error'; import { describe, it, expect, vi, beforeEach } from 'vitest'; const { legacyDatabaseAccess } = vi.hoisted(() => ({ @@ -208,7 +209,7 @@ describe('PlacesController (parity with the legacy /api/trips/:tripId/places rou // pins `url`, so the ZodValidationPipe rejects a urlless body before the // handler runs. it('maps a service { error, status } to the same response', async () => { - const s = svc({ importGoogleList: vi.fn().mockResolvedValue({ error: 'List is empty', status: 400 }) } as Partial); + const s = svc({ importGoogleList: vi.fn().mockRejectedValue(new DomainError(400, 'List is empty')) } as Partial); expect(await thrownAsync(() => new PlacesController(s, new RuntimeEnvService(), storageStub).importGoogle(user, '5', { url: 'http://x' }))).toEqual({ status: 400, body: { error: 'List is empty' } }); }); it('imports a naver list and returns the count + listName', async () => { @@ -579,7 +580,7 @@ describe('PlacesController (parity with the legacy /api/trips/:tripId/places rou it('GET /:id/image maps service error + returns photos', async () => { const s = svc({ searchImage: vi.fn().mockResolvedValue({ photos: [{ url: 'x' }] }) } as Partial); expect(await new PlacesController(s, new RuntimeEnvService(), storageStub).image(user, '5', '9')).toEqual({ photos: [{ url: 'x' }] }); - const e = svc({ searchImage: vi.fn().mockResolvedValue({ error: 'No key', status: 400 }) } as Partial); + const e = svc({ searchImage: vi.fn().mockRejectedValue(new DomainError(400, 'No key')) } as Partial); expect(await thrownAsync(() => new PlacesController(e, new RuntimeEnvService(), storageStub).image(user, '5', '9'))).toEqual({ status: 400, body: { error: 'No key' } }); }); diff --git a/server/tests/unit/nest/places.directions.test.ts b/server/tests/unit/nest/places.directions.test.ts index 30df5c0cb..7193ecb7d 100644 --- a/server/tests/unit/nest/places.directions.test.ts +++ b/server/tests/unit/nest/places.directions.test.ts @@ -6,6 +6,7 @@ * in a unit test. What is under test is the step between the two: which stops the link * yields, which of them get geocoded, and what a partly readable link leaves behind. */ +import { asLegacyResult } from '../../helpers/domain-error'; import { describe, it, expect, vi, beforeAll, beforeEach } from 'vitest'; vi.mock('../../../src/db/database', async () => { @@ -138,7 +139,7 @@ describe('PlacesService.importGoogleDirections', () => { const search = geocoder(); const url = 'https://www.google.com/maps/dir/Berlin/Dresden/@51.5,13.5,8z/' + 'data=!4m14!1m5!1m1!1s0x0:0x0!2m2!1d13.404954!2d52.520008!1m5!1m1!1s0x0:0x0!2m2!1d13.737262!2d51.050409'; - const result = await (await svc(search)).importGoogleDirections(tripId, url); + const result = await asLegacyResult((await svc(search)).importGoogleDirections(tripId, url)); expect('error' in result).toBe(false); if ('error' in result) return; @@ -150,10 +151,10 @@ describe('PlacesService.importGoogleDirections', () => { it('PLACES-DIR-002: a stop that is only a name is geocoded, and keeps the name from the link', async () => { const search = geocoder(); - const result = await (await svc(search)).importGoogleDirections( + const result = await asLegacyResult((await svc(search)).importGoogleDirections( tripId, 'https://www.google.com/maps/dir/Berlin/Dresden/Prague', - ); + )); expect('error' in result).toBe(false); if ('error' in result) return; @@ -163,10 +164,10 @@ describe('PlacesService.importGoogleDirections', () => { it('PLACES-DIR-003: a stop nobody can place is left out, not made up, and is counted', async () => { const search = geocoder(); - const result = await (await svc(search)).importGoogleDirections( + const result = await asLegacyResult((await svc(search)).importGoogleDirections( tripId, 'https://www.google.com/maps/dir/Berlin/Somewhere+Nobody+Knows/Prague', - ); + )); expect('error' in result).toBe(false); if ('error' in result) return; @@ -181,7 +182,7 @@ describe('PlacesService.importGoogleDirections', () => { if (query === 'Dresden') throw new Error('Nominatim 429'); return [hit(query, 52.52, 13.405)]; }) as unknown as OsmClient['searchNominatim']; - const result = await (await svc(search)).importGoogleDirections(tripId, 'https://www.google.com/maps/dir/Berlin/Dresden/Prague'); + const result = await asLegacyResult((await svc(search)).importGoogleDirections(tripId, 'https://www.google.com/maps/dir/Berlin/Dresden/Prague')); expect('error' in result).toBe(false); if ('error' in result) return; @@ -193,7 +194,7 @@ describe('PlacesService.importGoogleDirections', () => { createPlace(testDb, Number(tripId), { name: 'Berlin', lat: 52.520008, lng: 13.404954 }); const url = 'https://www.google.com/maps/dir/Berlin/Dresden/@51.5,13.5,8z/' + 'data=!4m14!1m5!1m1!1s0x0:0x0!2m2!1d13.404954!2d52.520008!1m5!1m1!1s0x0:0x0!2m2!1d13.737262!2d51.050409'; - const result = await (await svc(geocoder())).importGoogleDirections(tripId, url); + const result = await asLegacyResult((await svc(geocoder())).importGoogleDirections(tripId, url)); expect('error' in result).toBe(false); if ('error' in result) return; @@ -202,7 +203,7 @@ describe('PlacesService.importGoogleDirections', () => { }); it('PLACES-DIR-006: a link that is not Google\'s is refused after it is resolved', async () => { - const result = await (await svc(geocoder())).importGoogleDirections(tripId, 'https://evil.example.com/maps/dir/Berlin/Dresden'); + const result = await asLegacyResult((await svc(geocoder())).importGoogleDirections(tripId, 'https://evil.example.com/maps/dir/Berlin/Dresden')); expect(result).toEqual({ error: 'That link is not a Google Maps link.', status: 400 }); }); @@ -210,7 +211,7 @@ describe('PlacesService.importGoogleDirections', () => { safeFetchFollow.mockResolvedValue({ url: 'https://www.google.com/maps/dir/Berlin/Dresden', } as unknown as Response); - const result = await (await svc(geocoder())).importGoogleDirections(tripId, 'https://maps.app.goo.gl/abc123'); + const result = await asLegacyResult((await svc(geocoder())).importGoogleDirections(tripId, 'https://maps.app.goo.gl/abc123')); expect(safeFetchFollow).toHaveBeenCalledTimes(1); expect('error' in result).toBe(false); @@ -220,12 +221,12 @@ describe('PlacesService.importGoogleDirections', () => { it('PLACES-DIR-008: a blocked URL never reaches the parser', async () => { checkSsrf.mockResolvedValue({ allowed: false } as never); - const result = await (await svc(geocoder())).importGoogleDirections(tripId, 'https://www.google.com/maps/dir/Berlin/Dresden'); + const result = await asLegacyResult((await svc(geocoder())).importGoogleDirections(tripId, 'https://www.google.com/maps/dir/Berlin/Dresden')); expect(result).toEqual({ error: 'URL is not allowed', status: 400 }); }); it('PLACES-DIR-009: a link with nothing to read says what to do instead', async () => { - const result = await (await svc(geocoder())).importGoogleDirections(tripId, 'https://www.google.com/maps/dir/Berlin'); + const result = await asLegacyResult((await svc(geocoder())).importGoogleDirections(tripId, 'https://www.google.com/maps/dir/Berlin')); expect(result).toMatchObject({ status: 400 }); expect((result as { error: string }).error).toMatch(/Share button/); }); @@ -243,7 +244,7 @@ describe('PlacesService.importGoogleDirections', () => { (service as unknown as { maps: Partial }).maps.geocodeQuery = geocode as unknown as MapsService['geocodeQuery']; - await service.importGoogleDirections(tripId, 'https://www.google.com/maps/dir/Berlin/Dresden'); + await asLegacyResult(service.importGoogleDirections(tripId, 'https://www.google.com/maps/dir/Berlin/Dresden')); expect(geocode).toHaveBeenCalledWith('Berlin'); expect(geocode).toHaveBeenCalledWith('Dresden'); @@ -263,7 +264,7 @@ describe('PlacesService.importGoogleDirections', () => { url: 'https://www.google.com/maps/dir/52.52,13.405/51.05,13.74', } as never); - const result = await (await svc(geocoder())).importGoogleList(tripId, 'https://maps.app.goo.gl/aBcDeF12345'); + const result = await asLegacyResult((await svc(geocoder())).importGoogleList(tripId, 'https://maps.app.goo.gl/aBcDeF12345')); expect('error' in result).toBe(false); if ('error' in result) return; @@ -276,7 +277,7 @@ describe('PlacesService.importGoogleDirections', () => { it('PLACES-DIR-010: a route where only one stop can be placed is not half an import', async () => { const search = vi.fn(async () => []) as unknown as OsmClient['searchNominatim']; const url = 'https://www.google.com/maps/dir/52.52,13.405/Nowhere/Nowhere+Else'; - const result = await (await svc(search)).importGoogleDirections(tripId, url); + const result = await asLegacyResult((await svc(search)).importGoogleDirections(tripId, url)); expect(result).toEqual({ error: 'None of the stops in that link could be placed on the map.', status: 400 }); }); }); diff --git a/server/tests/unit/nest/places.service.test.ts b/server/tests/unit/nest/places.service.test.ts index 1bd741ffe..6d7983439 100644 --- a/server/tests/unit/nest/places.service.test.ts +++ b/server/tests/unit/nest/places.service.test.ts @@ -9,6 +9,7 @@ * service is constructed directly, no Nest container needed. External fetches * are mocked where needed. */ +import { asLegacyResult } from '../../helpers/domain-error'; import { describe, it, expect, vi, beforeAll, beforeEach, afterEach, afterAll } from 'vitest'; import type { PlacePhotoCacheService } from '../../../src/nest/place-photos/place-photo-cache.service'; import { UnsplashService } from '../../../src/nest/unsplash/unsplash.service'; @@ -1073,7 +1074,7 @@ describe('importGoogleList', () => { it('PLACE-SVC-026 — returns error when list ID cannot be extracted from URL', async () => { const { user } = createUser(testDb); const trip = createTrip(testDb, user.id); - const result = await svc.importGoogleList(String(trip.id), 'https://example.com/no-id-here') as any; + const result = await asLegacyResult(svc.importGoogleList(String(trip.id), 'https://example.com/no-id-here')) as any; expect(result.error).toMatch(/Could not extract list ID/); expect(result.status).toBe(400); }); @@ -1082,7 +1083,7 @@ describe('importGoogleList', () => { const { user } = createUser(testDb); const trip = createTrip(testDb, user.id); const url = 'https://www.google.com/maps/place/Eiffel+Tower/@48.8584,2.2945,17z/data=!3m1'; - const result = await svc.importGoogleList(String(trip.id), url) as any; + const result = await asLegacyResult(svc.importGoogleList(String(trip.id), url)) as any; expect(result.status).toBe(400); expect(result.error).toMatch(/single place/i); }); @@ -1092,7 +1093,7 @@ describe('importGoogleList', () => { const trip = createTrip(testDb, user.id); vi.stubGlobal('fetch', vi.fn().mockResolvedValue({ ok: false, text: async () => '', status: 502 })); const url = 'https://www.google.com/maps/placelists/list/ABC123DEF456'; - const result = await svc.importGoogleList(String(trip.id), url) as any; + const result = await asLegacyResult(svc.importGoogleList(String(trip.id), url)) as any; expect(result.error).toMatch(/Failed to fetch list/); expect(result.status).toBe(502); }); @@ -1113,7 +1114,7 @@ describe('importGoogleList', () => { })); const url = 'https://www.google.com/maps/placelists/list/ABC123DEF456'; - const result = await svc.importGoogleList(String(trip.id), url) as any; + const result = await asLegacyResult(svc.importGoogleList(String(trip.id), url)) as any; expect(result.listName).toBe('My Test List'); expect(result.places).toHaveLength(2); expect(result.places[0].name).toBe('Paris'); @@ -1132,7 +1133,7 @@ describe('importGoogleList', () => { ]], ]; vi.stubGlobal('fetch', vi.fn().mockResolvedValue({ ok: true, text: async () => 'prefix\n' + JSON.stringify(listPayload) })); - const result = await svc.importGoogleList(String(trip.id), 'https://www.google.com/maps/placelists/list/ABC123DEF456') as { places: { id: number }[] }; + const result = await asLegacyResult(svc.importGoogleList(String(trip.id), 'https://www.google.com/maps/placelists/list/ABC123DEF456')) as { places: { id: number }[] }; const row = await findRow(orm, Places, { id: result.places[0].id }) as Record; expect(row).toMatchObject({ trip_id: trip.id, name: 'London', description: null, lat: 51.5074, lng: -0.1278, @@ -1182,7 +1183,7 @@ describe('importGoogleList', () => { })); const url = 'https://www.google.com/maps/placelists/list/ABC123DEF456'; - const result = await svc.importGoogleList(String(trip.id), url) as any; + const result = await asLegacyResult(svc.importGoogleList(String(trip.id), url)) as any; expect(result.places).toHaveLength(1); expect(result.places[0].google_place_id).toBeNull(); @@ -1209,7 +1210,7 @@ describe('importGoogleList', () => { })); const url = 'https://www.google.com/maps/placelists/list/ABC123DEF456'; - const result = await svc.importGoogleList(String(trip.id), url) as any; + const result = await asLegacyResult(svc.importGoogleList(String(trip.id), url)) as any; const row = await findRow(orm, Places, { id: existing.id }) as any; expect(result.places).toHaveLength(0); @@ -1247,7 +1248,7 @@ describe('importGoogleList', () => { text: async () => 'prefix\n' + JSON.stringify(listPayload), })); - const result = await svc.importGoogleList(String(trip.id), 'https://www.google.com/maps/placelists/list/ABC123DEF456') as any; + const result = await asLegacyResult(svc.importGoogleList(String(trip.id), 'https://www.google.com/maps/placelists/list/ABC123DEF456')) as any; const other = await findRow(orm, Places, { id: namesake.id }) as any; expect(result.skipped).toBe(1); @@ -1270,7 +1271,7 @@ describe('importGoogleList', () => { const url = 'https://www.google.com/maps/placelists/list/ABC123DEF456'; respond(); - const first = await svc.importGoogleList(String(trip.id), url) as any; + const first = await asLegacyResult(svc.importGoogleList(String(trip.id), url)) as any; expect(first.places).toHaveLength(1); // What the reporter does: rename it to something they can actually read, and @@ -1278,7 +1279,7 @@ describe('importGoogleList', () => { await updateRows(orm, Places, { id: first.places[0].id }, { name: 'Saturday market', lat: 43.6, lng: -80.6 }); respond(); - const second = await svc.importGoogleList(String(trip.id), url) as any; + const second = await asLegacyResult(svc.importGoogleList(String(trip.id), url)) as any; expect(second.places).toHaveLength(0); expect(second.skipped).toBe(1); expect(({ c: await countRows(orm, Places, { trip: trip.id }) })).toEqual({ c: 1 }); @@ -1300,7 +1301,7 @@ describe('importGoogleList', () => { text: async () => 'prefix\n' + JSON.stringify(listPayload), })); - const result = await svc.importGoogleList(String(trip.id), 'https://www.google.com/maps/placelists/list/ABC123DEF456') as any; + const result = await asLegacyResult(svc.importGoogleList(String(trip.id), 'https://www.google.com/maps/placelists/list/ABC123DEF456')) as any; expect(result.places).toHaveLength(2); expect(result.skipped).toBe(0); }); @@ -1316,7 +1317,7 @@ describe('importGoogleList', () => { })); const url = 'https://www.google.com/maps/placelists/list/ABC123DEF456'; - const result = await svc.importGoogleList(String(trip.id), url) as any; + const result = await asLegacyResult(svc.importGoogleList(String(trip.id), url)) as any; expect(result.error).toBeDefined(); expect(result.status).toBe(400); }); @@ -1332,7 +1333,7 @@ describe('searchImage', () => { it('PLACE-SVC-030 — returns 404 when place does not exist', async () => { const { user } = createUser(testDb); const trip = createTrip(testDb, user.id); - const result = await svc.searchImage(String(trip.id), '99999', user.id) as any; + const result = await asLegacyResult(svc.searchImage(String(trip.id), '99999', user.id)) as any; expect(result.error).toBeDefined(); expect(result.status).toBe(404); }); @@ -1341,12 +1342,8 @@ describe('searchImage', () => { const { user } = createUser(testDb); const trip = createTrip(testDb, user.id); const place = createPlace(testDb, trip.id); - const result: Awaited> = await svc.searchImage(String(trip.id), `${place.id} `, user.id); - expect('error' in result).toBe(true); - if ('error' in result) { - expect(result.error).toBeDefined(); - expect(result.status).toBe(404); - } + const result = await asLegacyResult(svc.searchImage(String(trip.id), `${place.id} `, user.id)); + expect(result).toEqual({ error: 'Place not found', status: 404 }); }); it('PLACE-SVC-031 — searches Unsplash without a stored API key', async () => { @@ -1363,7 +1360,7 @@ describe('searchImage', () => { status: 200, })); - const result = await svc.searchImage(String(trip.id), String(place.id), user.id) as any; + const result = await asLegacyResult(svc.searchImage(String(trip.id), String(place.id), user.id)) as any; expect(result.photos).toHaveLength(1); const [url] = (fetch as any).mock.calls[0]; expect(url).toContain('https://unsplash.com/napi/search/photos?'); @@ -1384,7 +1381,7 @@ describe('searchImage', () => { status: 200, })); - const result = await svc.searchImage(String(trip.id), String(place.id), user.id) as any; + const result = await asLegacyResult(svc.searchImage(String(trip.id), String(place.id), user.id)) as any; expect(result.photos).toHaveLength(1); expect(result.photos[0].id).toBe('photo1'); expect(result.photos[0].url).toBe('https://img.example.com/1'); @@ -2083,7 +2080,7 @@ describe('importGoogleList provider payload', () => { const { user } = createUser(testDb); const trip = createTrip(testDb, user.id); vi.stubGlobal('fetch', vi.fn().mockResolvedValue({ ok: true, text: async () => 'prefix\nnot-json-at-all' })); - const result = await svc.importGoogleList(String(trip.id), 'https://www.google.com/maps/placelists/list/ABC123DEF456') as any; + const result = await asLegacyResult(svc.importGoogleList(String(trip.id), 'https://www.google.com/maps/placelists/list/ABC123DEF456')) as any; expect(result).toEqual({ error: 'Invalid list data received from Google Maps', status: 400 }); }); @@ -2091,7 +2088,7 @@ describe('importGoogleList provider payload', () => { const { user } = createUser(testDb); const trip = createTrip(testDb, user.id); vi.stubGlobal('fetch', vi.fn().mockResolvedValue({ ok: true, text: async () => 'prefix\n{"unexpected":true}' })); - const result = await svc.importGoogleList(String(trip.id), 'https://www.google.com/maps/placelists/list/ABC123DEF456') as any; + const result = await asLegacyResult(svc.importGoogleList(String(trip.id), 'https://www.google.com/maps/placelists/list/ABC123DEF456')) as any; expect(result).toEqual({ error: 'Invalid list data received from Google Maps', status: 400 }); }); @@ -2103,7 +2100,7 @@ describe('importGoogleList provider payload', () => { headers: { get: () => null }, // chunked: the declared check cannot help text: async () => 'x'.repeat(9 * 1024 * 1024), })); - const result = await svc.importGoogleList(String(trip.id), 'https://www.google.com/maps/placelists/list/ABC123DEF456') as any; + const result = await asLegacyResult(svc.importGoogleList(String(trip.id), 'https://www.google.com/maps/placelists/list/ABC123DEF456')) as any; expect(result).toEqual({ error: 'Failed to fetch list from Google Maps', status: 502 }); }); @@ -2116,7 +2113,7 @@ describe('importGoogleList provider payload', () => { headers: { get: (h: string) => (h.toLowerCase() === 'content-length' ? String(20 * 1024 * 1024) : null) }, text, })); - const result = await svc.importGoogleList(String(trip.id), 'https://www.google.com/maps/placelists/list/ABC123DEF456') as any; + const result = await asLegacyResult(svc.importGoogleList(String(trip.id), 'https://www.google.com/maps/placelists/list/ABC123DEF456')) as any; expect(result).toEqual({ error: 'Failed to fetch list from Google Maps', status: 502 }); expect(text).not.toHaveBeenCalled(); }); @@ -2140,7 +2137,7 @@ describe('importNaverList provider payload', () => { headers: { get: (h: string) => (h.toLowerCase() === 'content-length' ? String(20 * 1024 * 1024) : null) }, text, })); - const result = await svc.importNaverList(String(trip.id), FOLDER_URL) as any; + const result = await asLegacyResult(svc.importNaverList(String(trip.id), FOLDER_URL)) as any; expect(result).toEqual({ error: 'Failed to fetch list from Naver Maps', status: 502 }); expect(text).not.toHaveBeenCalled(); }); @@ -2153,7 +2150,7 @@ describe('importNaverList provider payload', () => { headers: { get: () => null }, // chunked: the declared check cannot help text: async () => 'x'.repeat(9 * 1024 * 1024), })); - const result = await svc.importNaverList(String(trip.id), FOLDER_URL) as any; + const result = await asLegacyResult(svc.importNaverList(String(trip.id), FOLDER_URL)) as any; expect(result).toEqual({ error: 'Failed to fetch list from Naver Maps', status: 502 }); }); @@ -2161,7 +2158,7 @@ describe('importNaverList provider payload', () => { const { user } = createUser(testDb); const trip = createTrip(testDb, user.id); vi.stubGlobal('fetch', vi.fn().mockResolvedValue({ ok: true, text: async () => 'not-json-at-all' })); - const result = await svc.importNaverList(String(trip.id), FOLDER_URL) as any; + const result = await asLegacyResult(svc.importNaverList(String(trip.id), FOLDER_URL)) as any; expect(result).toEqual({ error: 'Invalid list data received from Naver Maps', status: 400 }); }); @@ -2175,7 +2172,7 @@ describe('importNaverList provider payload', () => { bookmarkList: [{ name: 'Gyeongbokgung', px: 126.977, py: 37.5796, memo: null, address: 'Sejongno' }], }), })); - const result = await svc.importNaverList(String(trip.id), FOLDER_URL) as any; + const result = await asLegacyResult(svc.importNaverList(String(trip.id), FOLDER_URL)) as any; expect(result.listName).toBe('Seoul'); expect(result.places).toHaveLength(1); expect(result.places[0].name).toBe('Gyeongbokgung'); @@ -2194,7 +2191,7 @@ describe('importNaverList provider payload', () => { bookmarkList: [{ name: 'Gyeongbokgung', px: 126.977, py: 37.5796, memo: 'A palace', address: 'Sejongno' }], }), })); - const result = await svc.importNaverList(String(trip.id), FOLDER_URL) as { places: { id: number }[] }; + const result = await asLegacyResult(svc.importNaverList(String(trip.id), FOLDER_URL)) as { places: { id: number }[] }; const row = await findRow(orm, Places, { id: result.places[0].id }) as Record; expect(row).toMatchObject({ trip_id: trip.id, name: 'Gyeongbokgung', description: null, lat: 37.5796, lng: 126.977, diff --git a/server/tests/unit/nest/trips.controller.test.ts b/server/tests/unit/nest/trips.controller.test.ts index 5779b85b8..7da98b924 100644 --- a/server/tests/unit/nest/trips.controller.test.ts +++ b/server/tests/unit/nest/trips.controller.test.ts @@ -1,3 +1,4 @@ +import { DomainError } from '../../../src/nest/common/domain-error'; import { describe, it, expect, vi, beforeEach } from 'vitest'; import type { UnsplashService } from '../../../src/nest/unsplash/unsplash.service'; import { RuntimeEnvService } from '../../../src/nest/app-config/runtime-env.service'; @@ -128,14 +129,14 @@ describe('TripsController (parity with the legacy /api/trips route)', async () = }); it('sends an absent query as "" so the service keeps owning the empty-query 400', async () => { - const searchCoverImages = vi.fn().mockResolvedValue({ error: 'Search query is required', status: 400 }); + const searchCoverImages = vi.fn().mockRejectedValue(new DomainError(400, 'Search query is required')); const s = svc({ searchCoverImages } as Partial); expect(await thrownAsync(() => tc(s).coverImages(user, undefined))).toEqual({ status: 400, body: { error: 'Search query is required' } }); expect(searchCoverImages).toHaveBeenCalledWith('', 1); }); it('keeps the upstream status instead of flattening it into the catch-all 500', async () => { - const s = svc({ searchCoverImages: vi.fn().mockResolvedValue({ error: 'Rate limit reached', status: 429 }) } as Partial); + const s = svc({ searchCoverImages: vi.fn().mockRejectedValue(new DomainError(429, 'Rate limit reached')) } as Partial); // A 429 that arrives as a 500 makes the client retry immediately and burn // the rest of the hourly Unsplash quota. expect(await thrownAsync(() => tc(s).coverImages(user, 'kyoto'))).toEqual({ status: 429, body: { error: 'Rate limit reached' } }); diff --git a/server/tests/unit/nest/unsplash/unsplash.service.test.ts b/server/tests/unit/nest/unsplash/unsplash.service.test.ts index d1c19ca28..51556fd9f 100644 --- a/server/tests/unit/nest/unsplash/unsplash.service.test.ts +++ b/server/tests/unit/nest/unsplash/unsplash.service.test.ts @@ -1,3 +1,4 @@ +import { asLegacyResult } from '../../../helpers/domain-error'; import { describe, it, expect, vi, afterEach } from 'vitest'; import fs from 'fs'; import path from 'path'; @@ -33,7 +34,8 @@ const coverFx = makeStorageFixture('covers/'); const appSettingsStub = { getValue: mockGetValue } as unknown as AppSettingsRepository; const usersStub = { getApiKeyColumn: mockGetApiKeyColumn } as unknown as UsersRepository; const svc = new UnsplashService(appSettingsStub, usersStub, new RuntimeEnvService(), coverFx.storage); -const searchUnsplashPhotos = svc.searchUnsplashPhotos.bind(svc); +const searchUnsplashPhotos = (...args: Parameters) => + asLegacyResult(svc.searchUnsplashPhotos(...args)); const getUnsplashKey = svc.getUnsplashKey.bind(svc); const saveUnsplashCover = svc.saveUnsplashCover.bind(svc); const isUnsplashCoverUrl = svc.isUnsplashCoverUrl.bind(svc); diff --git a/server/tests/unit/nest/vacay.controller.test.ts b/server/tests/unit/nest/vacay.controller.test.ts index 852b2af42..a4768474d 100644 --- a/server/tests/unit/nest/vacay.controller.test.ts +++ b/server/tests/unit/nest/vacay.controller.test.ts @@ -1,3 +1,4 @@ +import { DomainError } from '../../../src/nest/common/domain-error'; import { describe, it, expect, vi } from 'vitest'; import { HttpException } from '@nestjs/common'; import { VacayController } from '../../../src/nest/vacay/vacay.controller'; @@ -83,7 +84,7 @@ describe('VacayController (parity with the legacy /api/addons/vacay route)', () }); it('maps a sendInvite error to its status', () => { - const sendInvite = vi.fn().mockReturnValue({ error: 'Already in a plan', status: 409 }); + const sendInvite = vi.fn().mockRejectedValue(new DomainError(409, 'Already in a plan')); return thrown(() => makeController({ ...planBase, sendInvite }).invite(user, { user_id: 2 })).then((r) => expect(r).toEqual({ status: 409, body: { error: 'Already in a plan' } })); }); @@ -95,7 +96,7 @@ describe('VacayController (parity with the legacy /api/addons/vacay route)', () }); it('maps an acceptInvite error', () => { - const acceptInvite = vi.fn().mockReturnValue({ error: 'Invite not found', status: 404 }); + const acceptInvite = vi.fn().mockRejectedValue(new DomainError(404, 'Invite not found')); return thrown(() => makeController({ acceptInvite }).acceptInvite(user, { plan_id: 5 })).then((r) => expect(r).toEqual({ status: 404, body: { error: 'Invite not found' } })); }); @@ -195,7 +196,7 @@ describe('VacayController (parity with the legacy /api/addons/vacay route)', () describe('public holidays', () => { it('502 when the upstream country lookup fails', () => { - const getCountries = vi.fn().mockResolvedValue({ error: 'upstream down' }); + const getCountries = vi.fn().mockRejectedValue(new DomainError(502, 'upstream down')); return thrown(() => makeController({ getCountries }).holidayCountries()).then((r) => expect(r).toEqual({ status: 502, body: { error: 'upstream down' } })); }); @@ -206,7 +207,7 @@ describe('VacayController (parity with the legacy /api/addons/vacay route)', () }); it('502 when the holidays lookup fails', () => { - const getHolidays = vi.fn().mockResolvedValue({ error: 'upstream down' }); + const getHolidays = vi.fn().mockRejectedValue(new DomainError(502, 'upstream down')); return thrown(() => makeController({ getHolidays }).holidays('2026', 'DE')).then((r) => expect(r).toEqual({ status: 502, body: { error: 'upstream down' } })); }); @@ -225,7 +226,7 @@ describe('VacayController (parity with the legacy /api/addons/vacay route)', () }); it('maps a shareCalendar error to its status', () => { - const shareCalendar = vi.fn().mockReturnValue({ error: 'Already shared', status: 400 }); + const shareCalendar = vi.fn().mockRejectedValue(new DomainError(400, 'Already shared')); return thrown(() => makeController({ shareCalendar }).share(user, { user_id: 2 })).then((r) => expect(r).toEqual({ status: 400, body: { error: 'Already shared' } })); }); diff --git a/server/tests/unit/nest/vacay.service.test.ts b/server/tests/unit/nest/vacay.service.test.ts index df44cf52b..410a7560c 100644 --- a/server/tests/unit/nest/vacay.service.test.ts +++ b/server/tests/unit/nest/vacay.service.test.ts @@ -1,3 +1,4 @@ +import { asLegacyResult } from '../../helpers/domain-error'; import { describe, it, expect, vi, beforeAll, beforeEach, afterAll } from 'vitest'; // ── DB setup (real in-memory SQLite) ───────────────────────────────────────── @@ -902,7 +903,7 @@ describe('acceptInvite', () => { await svc.getOwnPlan(invitee.id); // ensure own plan exists for data migration path await insertMember(ownerPlan.id, invitee.id, 'pending'); - const result = await svc.acceptInvite(invitee.id, ownerPlan.id, undefined); + const result = await asLegacyResult(svc.acceptInvite(invitee.id, ownerPlan.id, undefined)); expect(result.error).toBeUndefined(); const row = await storedFields(VacayPlanMembers, { plan: ownerPlan.id, user: invitee.id }, ['status']) as { status: string } | undefined; @@ -912,7 +913,7 @@ describe('acceptInvite', () => { it('VACAY-SVC-038: returns 404 error when there is no pending invite', async () => { const { user } = createUser(testDb); - const result = await svc.acceptInvite(user.id, 99999, undefined); + const result = await asLegacyResult(svc.acceptInvite(user.id, 99999, undefined)); expect(result.status).toBe(404); expect(result.error).toBeDefined(); @@ -924,7 +925,7 @@ describe('acceptInvite', () => { await svc.getOwnPlan(invitee.id); await insertMember(ownerPlan.id, invitee.id, 'pending'); - await svc.acceptInvite(invitee.id, ownerPlan.id, undefined); + await asLegacyResult(svc.acceptInvite(invitee.id, ownerPlan.id, undefined)); const active = await svc.getActivePlan(invitee.id); expect(active.id).toBe(ownerPlan.id); @@ -1013,9 +1014,9 @@ describe('getAvailableUsers', () => { // The picker is only a list. The id comes back from the client, and the MCP // tools reach the same two methods, so refusing has to happen here. - const invited = await svc.sendInvite(plan.id, owner.id, 'owner', 'owner@example.test', guest.id); + const invited = await asLegacyResult(svc.sendInvite(plan.id, owner.id, 'owner', 'owner@example.test', guest.id)); expect(invited.error).toBe('User not found'); - const shared = await svc.shareCalendar(owner.id, 'owner@example.test', guest.id); + const shared = await asLegacyResult(svc.shareCalendar(owner.id, 'owner@example.test', guest.id)); expect(shared.error).toBe('User not found'); }); }); @@ -1463,7 +1464,7 @@ describe('shareCalendar', () => { const { user: owner } = await setupUserWithPlan(); const { user: target } = createUser(testDb); - const result = await svc.shareCalendar(owner.id, owner.email, target.id); + const result = await asLegacyResult(svc.shareCalendar(owner.id, owner.email, target.id)); expect(result.error).toBeUndefined(); const row = await storedRow(VacayShares, { owner: owner.id, user: target.id }); @@ -1473,7 +1474,7 @@ describe('shareCalendar', () => { it('VACAY-SVC-049: returns 400 when sharing with yourself', async () => { const { user: owner } = await setupUserWithPlan(); - const result = await svc.shareCalendar(owner.id, owner.email, owner.id); + const result = await asLegacyResult(svc.shareCalendar(owner.id, owner.email, owner.id)); expect(result).toEqual({ error: 'Cannot share with yourself', status: 400 }); }); @@ -1481,7 +1482,7 @@ describe('shareCalendar', () => { it('VACAY-SVC-050: returns 404 when the target user does not exist', async () => { const { user: owner } = await setupUserWithPlan(); - const result = await svc.shareCalendar(owner.id, owner.email, 99999); + const result = await asLegacyResult(svc.shareCalendar(owner.id, owner.email, 99999)); expect(result).toEqual({ error: 'User not found', status: 404 }); }); @@ -1489,9 +1490,9 @@ describe('shareCalendar', () => { it('VACAY-SVC-051: returns 400 when the share already exists', async () => { const { user: owner } = await setupUserWithPlan(); const { user: target } = createUser(testDb); - await svc.shareCalendar(owner.id, owner.email, target.id); + await asLegacyResult(svc.shareCalendar(owner.id, owner.email, target.id)); - const result = await svc.shareCalendar(owner.id, owner.email, target.id); + const result = await asLegacyResult(svc.shareCalendar(owner.id, owner.email, target.id)); expect(result).toEqual({ error: 'Already shared', status: 400 }); }); @@ -1501,7 +1502,7 @@ describe('shareCalendar', () => { const { user: member } = createUser(testDb); await insertMember(plan.id, member.id, 'accepted'); - const result = await svc.shareCalendar(owner.id, owner.email, member.id); + const result = await asLegacyResult(svc.shareCalendar(owner.id, owner.email, member.id)); expect(result).toEqual({ error: 'User is already in your calendar', status: 400 }); }); @@ -1511,7 +1512,7 @@ describe('listShares', () => { it('VACAY-SVC-053: outgoing rows carry the target user info', async () => { const { user: owner } = await setupUserWithPlan(); const { user: target } = createUser(testDb); - await svc.shareCalendar(owner.id, owner.email, target.id); + await asLegacyResult(svc.shareCalendar(owner.id, owner.email, target.id)); const result = await svc.listShares(owner.id); @@ -1528,7 +1529,7 @@ describe('listShares', () => { const { user: owner, plan } = await setupUserWithPlan(); await svc.setUserColor(owner.id, plan.id, '#ef4444', undefined); const { user: viewer } = await setupUserWithPlan(); - await svc.shareCalendar(owner.id, owner.email, viewer.id); + await asLegacyResult(svc.shareCalendar(owner.id, owner.email, viewer.id)); const result = await svc.listShares(viewer.id); @@ -1547,8 +1548,8 @@ describe('listShares', () => { const { user: viewer } = await setupUserWithPlan(); // viewer's own color is #6366f1 const { user: owner1 } = await setupUserWithPlan(); // default #6366f1 const { user: owner2 } = await setupUserWithPlan(); // default #6366f1 - await svc.shareCalendar(owner1.id, owner1.email, viewer.id); - await svc.shareCalendar(owner2.id, owner2.email, viewer.id); + await asLegacyResult(svc.shareCalendar(owner1.id, owner1.email, viewer.id)); + await asLegacyResult(svc.shareCalendar(owner2.id, owner2.email, viewer.id)); const { incoming } = await svc.listShares(viewer.id); @@ -1564,7 +1565,7 @@ describe('removeShare', () => { it('VACAY-SVC-056: the owner can revoke their share', async () => { const { user: owner } = await setupUserWithPlan(); const { user: viewer } = createUser(testDb); - await svc.shareCalendar(owner.id, owner.email, viewer.id); + await asLegacyResult(svc.shareCalendar(owner.id, owner.email, viewer.id)); const shareId = (await svc.listShares(owner.id)).outgoing[0].id as number; expect(await svc.removeShare(shareId, owner.id)).toBe(true); @@ -1575,7 +1576,7 @@ describe('removeShare', () => { it('VACAY-SVC-057: the recipient can remove a share they received', async () => { const { user: owner } = await setupUserWithPlan(); const { user: viewer } = createUser(testDb); - await svc.shareCalendar(owner.id, owner.email, viewer.id); + await asLegacyResult(svc.shareCalendar(owner.id, owner.email, viewer.id)); const shareId = (await svc.listShares(viewer.id)).incoming[0].id; expect(await svc.removeShare(shareId, viewer.id)).toBe(true); @@ -1585,7 +1586,7 @@ describe('removeShare', () => { const { user: owner } = await setupUserWithPlan(); const { user: viewer } = createUser(testDb); const { user: stranger } = createUser(testDb); - await svc.shareCalendar(owner.id, owner.email, viewer.id); + await asLegacyResult(svc.shareCalendar(owner.id, owner.email, viewer.id)); const shareId = (await svc.listShares(owner.id)).outgoing[0].id as number; expect(await svc.removeShare(shareId, stranger.id)).toBe(false); @@ -1600,7 +1601,7 @@ describe('setShareHidden', () => { it('VACAY-SVC-059: the recipient can hide and unhide the shared calendar', async () => { const { user: owner } = await setupUserWithPlan(); const { user: viewer } = createUser(testDb); - await svc.shareCalendar(owner.id, owner.email, viewer.id); + await asLegacyResult(svc.shareCalendar(owner.id, owner.email, viewer.id)); const shareId = (await svc.listShares(viewer.id)).incoming[0].id; expect(await svc.setShareHidden(shareId, viewer.id, true)).toBe(true); @@ -1616,7 +1617,7 @@ describe('setShareHidden', () => { it('VACAY-SVC-060: the owner cannot toggle the recipient hidden flag', async () => { const { user: owner } = await setupUserWithPlan(); const { user: viewer } = createUser(testDb); - await svc.shareCalendar(owner.id, owner.email, viewer.id); + await asLegacyResult(svc.shareCalendar(owner.id, owner.email, viewer.id)); const shareId = (await svc.listShares(owner.id)).outgoing[0].id as number; expect(await svc.setShareHidden(shareId, owner.id, true)).toBe(false); @@ -1631,7 +1632,7 @@ describe('getShareAvailableUsers', () => { const { user: member } = createUser(testDb); await insertMember(plan.id, member.id, 'accepted'); const { user: shared } = createUser(testDb); - await svc.shareCalendar(owner.id, owner.email, shared.id); + await asLegacyResult(svc.shareCalendar(owner.id, owner.email, shared.id)); const { user: unrelated } = createUser(testDb); const ids = (await svc.getShareAvailableUsers(owner.id) as { id: number }[]).map(u => u.id); @@ -1652,7 +1653,7 @@ describe('getSharedCalendars', () => { await svc.toggleEntry(owner.id, plan.id, '2025-06-10', 1); await svc.toggleEntry(owner.id, plan.id, '2025-06-11', 0.5); await svc.toggleEntry(member.id, plan.id, '2025-06-12', 1); - await svc.shareCalendar(owner.id, owner.email, viewer.id); + await asLegacyResult(svc.shareCalendar(owner.id, owner.email, viewer.id)); const calendars = await svc.getSharedCalendars(viewer.id, '2025'); @@ -1671,7 +1672,7 @@ describe('getSharedCalendars', () => { const { user: viewer } = createUser(testDb); await updateRows(await orm(), VacayPlans, { id: plan.id }, { company_holidays_enabled: 0 }); await svc.toggleCompanyHoliday(plan.id, '2025-12-24', 'Christmas Eve', undefined); - await svc.shareCalendar(owner.id, owner.email, viewer.id); + await asLegacyResult(svc.shareCalendar(owner.id, owner.email, viewer.id)); const calendars = await svc.getSharedCalendars(viewer.id, '2025'); @@ -1683,7 +1684,7 @@ describe('getSharedCalendars', () => { const { user: viewer } = createUser(testDb); await updateRows(await orm(), VacayPlans, { id: plan.id }, { company_holidays_enabled: 1 }); await svc.toggleCompanyHoliday(plan.id, '2025-12-24', 'Christmas Eve', undefined); - await svc.shareCalendar(owner.id, owner.email, viewer.id); + await asLegacyResult(svc.shareCalendar(owner.id, owner.email, viewer.id)); const calendars = await svc.getSharedCalendars(viewer.id, '2025'); @@ -1711,7 +1712,7 @@ describe('getSharedCalendars', () => { await insertMember(hostPlan.id, owner.id, 'accepted'); const { user: viewer } = createUser(testDb); await svc.toggleEntry(owner.id, hostPlan.id, '2025-03-03', 1); - await svc.shareCalendar(owner.id, owner.email, viewer.id); + await asLegacyResult(svc.shareCalendar(owner.id, owner.email, viewer.id)); const calendars = await svc.getSharedCalendars(viewer.id, '2025'); @@ -1812,9 +1813,9 @@ describe('quirk fixes', () => { vi.stubGlobal('fetch', fetchMock); const fresh = await freshVacayService(); - expect(await fresh.getCountries()).toEqual({ error: 'Failed to fetch countries' }); + expect(await asLegacyResult(fresh.getCountries())).toEqual({ error: 'Failed to fetch countries', status: 502 }); // Nothing cached: a retry hits the network again. - expect(await fresh.getCountries()).toEqual({ error: 'Failed to fetch countries' }); + expect(await asLegacyResult(fresh.getCountries())).toEqual({ error: 'Failed to fetch countries', status: 502 }); expect(fetchMock).toHaveBeenCalledTimes(2); expect(fetchMock).toHaveBeenCalledWith(expect.any(String), expect.objectContaining({ signal: expect.any(AbortSignal) })); }); @@ -1825,7 +1826,7 @@ describe('quirk fixes', () => { const fresh = await freshVacayService(); for (const [year, country] of [['../../..', 'DE'], ['2026', 'DE/../../x'], ['20xx', 'DE'], ['2026', 'DEU']]) { - expect(await fresh.getHolidays(year, country)).toEqual({ error: 'Failed to fetch holidays' }); + expect(await asLegacyResult(fresh.getHolidays(year, country))).toEqual({ error: 'Failed to fetch holidays', status: 502 }); } expect(fetchMock).not.toHaveBeenCalled(); }); @@ -1835,8 +1836,9 @@ describe('quirk fixes', () => { vi.stubGlobal('fetch', fetchMock); const fresh = await freshVacayService(); - expect(await fresh.getSchoolHolidayRegions('DE&countryIsoCode=FR')).toEqual({ + expect(await asLegacyResult(fresh.getSchoolHolidayRegions('DE&countryIsoCode=FR'))).toEqual({ error: 'Failed to fetch school holiday regions', + status: 502, }); expect(fetchMock).not.toHaveBeenCalled(); }); @@ -1850,8 +1852,8 @@ describe('quirk fixes', () => { vi.stubGlobal('fetch', fetchMock); const fresh = await freshVacayService(); - expect(await fresh.getCountries()).toEqual({ error: 'Failed to fetch countries' }); - expect(await fresh.getHolidays('2026', 'DE')).toEqual({ error: 'Failed to fetch holidays' }); + expect(await asLegacyResult(fresh.getCountries())).toEqual({ error: 'Failed to fetch countries', status: 502 }); + expect(await asLegacyResult(fresh.getHolidays('2026', 'DE'))).toEqual({ error: 'Failed to fetch holidays', status: 502 }); }); it('VACAY-SVC-070d: a chunked provider body past the cap reads as the usual fetch error', async () => { @@ -1876,8 +1878,8 @@ describe('quirk fixes', () => { vi.stubGlobal('fetch', fetchMock); const fresh = await freshVacayService(); - expect(await fresh.getHolidays('2026', 'DE')).toEqual({ error: 'Failed to fetch holidays' }); - expect(await fresh.getCountries()).toEqual({ error: 'Failed to fetch countries' }); + expect(await asLegacyResult(fresh.getHolidays('2026', 'DE'))).toEqual({ error: 'Failed to fetch holidays', status: 502 }); + expect(await asLegacyResult(fresh.getCountries())).toEqual({ error: 'Failed to fetch countries', status: 502 }); }); it('VACAY-SVC-071: applyHolidayCalendars honors the cache TTL', async () => {