Compare commits

..

3 Commits

Author SHA1 Message Date
jubnl ee31c78db8 fix(maps): null stale proxy image_url entries instead of writing unbacked proxy URLs
Migration 107 and the previous fix both wrote /api/maps/place-photo/<id>/bytes
into places.image_url without ever fetching the photo bytes. photoService
short-circuits on that URL prefix and hits /bytes directly, which 404s because
nothing is on disk.

- Add migration to null proxy image_url rows with no backing google_place_photo_meta
  entry — restores the normal fetch-and-cache flow for affected rows
- Fix the previous legacy-URL migration to null instead of rewrite, so fresh
  installs don't hit the same 404 path

Fixes #770 (follow-up)
2026-04-21 00:46:29 +02:00
jubnl edf14e2ebc test(maps): update getPlacePhoto stubs to use text() instead of json()
mapsService now reads the details response body via .text() before parsing,
so test stubs need text() rather than json().
2026-04-21 00:16:54 +02:00
jubnl 2aad8f465c fix(maps): prevent server crash when legacy Google photo URLs are stored as placeIds
Migration 107 only rewrote image_url rows matching /places/%/photos/%; URLs using
the /place-photos/ or /places/<opaque> paths survived the upgrade and were passed
verbatim to the Places API, producing a malformed request whose empty/HTML response
body threw SyntaxError before detailsRes.ok was checked. The resulting rejection was
leaked by placePhotoCache.setInFlight via an unhandled .finally() chain, triggering
Node 22's default unhandledRejection=throw and terminating the process.

- placePhotoCache: add .catch() after .finally() to prevent unhandled rejection crash
- mapsService: reject URL-shaped placeIds early; read response as text before JSON.parse
- migrations: add migration to rewrite remaining googleusercontent/places.googleapis URLs
- MapView/MapViewGL: prefer stable proxy URL form of image_url before google_place_id

Fixes #770
2026-04-21 00:13:35 +02:00
6 changed files with 69 additions and 11 deletions
+5 -1
View File
@@ -477,7 +477,11 @@ export const MapView = memo(function MapView({
cleanups.push(onThumbReady(cacheKey, thumb => setThumb(cacheKey, thumb)))
if (!cached && !isLoading(cacheKey)) {
const photoId = place.image_url || place.google_place_id || place.osm_id
const photoId =
(place.image_url?.startsWith('/api/maps/place-photo/') ? place.image_url : null)
|| place.google_place_id
|| place.osm_id
|| place.image_url
if (photoId || (place.lat && place.lng)) {
fetchPhoto(cacheKey, photoId || `coords:${place.lat}:${place.lng}`, place.lat, place.lng, place.name)
}
+5 -1
View File
@@ -366,7 +366,11 @@ export function MapViewGL({
}
cleanups.push(onThumbReady(cacheKey, thumb => setThumb(cacheKey, thumb)))
if (!cached && !isLoading(cacheKey)) {
const photoId = place.image_url || place.google_place_id || place.osm_id
const photoId =
(place.image_url?.startsWith('/api/maps/place-photo/') ? place.image_url : null)
|| place.google_place_id
|| place.osm_id
|| place.image_url
if (photoId || (place.lat && place.lng)) {
fetchPhoto(cacheKey, photoId || `coords:${place.lat}:${place.lng}`, place.lat, place.lng, place.name)
}
+40
View File
@@ -1906,6 +1906,46 @@ function runMigrations(db: Database.Database): void {
CREATE INDEX IF NOT EXISTS idx_day_accommodations_end_day_id ON day_accommodations(end_day_id);
`);
},
// Migration: null out proxy image_url entries that have no backing disk cache.
// Migrations 107 and the migration below wrote /api/maps/place-photo/<id>/bytes
// into places.image_url without actually fetching/caching the photo bytes. The
// photoService short-circuits on that prefix and hits /bytes directly → 404.
// Rows with a confirmed disk cache entry in google_place_photo_meta are left alone;
// only stale proxy URLs (never actually fetched) are cleared so the normal
// fetch-and-cache flow can repopulate them.
() => {
db.exec(`
UPDATE places
SET image_url = NULL, updated_at = CURRENT_TIMESTAMP
WHERE image_url LIKE '/api/maps/place-photo/%/bytes'
AND google_place_id IS NOT NULL
AND NOT EXISTS (
SELECT 1 FROM google_place_photo_meta
WHERE place_id = places.google_place_id
AND error_at IS NULL
)
`);
},
// Migration: clear legacy Google photo URLs missed by Migration 107.
// Migration 107 matched /places/%/photos/% only; lh3.googleusercontent.com URLs use
// /place-photos/ or /places/<opaque-id> paths and were skipped. NULL those stale URLs
// so the normal fetch-and-cache flow repopulates image_url with a real proxy URL.
() => {
db.exec(`
UPDATE places
SET image_url = NULL,
updated_at = CURRENT_TIMESTAMP
WHERE image_url IS NOT NULL
AND image_url != ''
AND image_url NOT LIKE '/api/maps/place-photo/%'
AND (
image_url LIKE 'http://%googleusercontent.com/%'
OR image_url LIKE 'https://%googleusercontent.com/%'
OR image_url LIKE 'http://%places.googleapis.com/%'
OR image_url LIKE 'https://%places.googleapis.com/%'
)
`);
},
];
if (currentVersion < migrations.length) {
+11 -3
View File
@@ -648,6 +648,12 @@ export async function getPlacePhoto(
return null;
}
// Reject URL-shaped placeIds — legacy DBs may store raw photo URLs in image_url
if (/^https?:\/\//i.test(placeId)) {
placePhotoCache.markError(placeId);
return null;
}
// Google Photos — fetch details to get photo name
const detailsRes = await googleFetch(`https://places.googleapis.com/v1/places/${placeId}`, `getPlacePhoto/details(${placeId})`, {
headers: {
@@ -655,13 +661,15 @@ export async function getPlacePhoto(
'X-Goog-FieldMask': 'photos',
},
});
const details = await detailsRes.json() as GooglePlaceDetails & { error?: { message?: string } };
const body = await detailsRes.text();
if (!detailsRes.ok) {
console.error('Google Places photo details error:', details.error?.message || detailsRes.status);
console.error('Google Places photo details error:', detailsRes.status, body.slice(0, 200));
placePhotoCache.markError(placeId);
return null;
}
let details: GooglePlaceDetails & { error?: { message?: string } };
try { details = body ? JSON.parse(body) : { photos: [] }; }
catch { placePhotoCache.markError(placeId); return null; }
if (!details.photos?.length) {
placePhotoCache.markError(placeId);
+3 -1
View File
@@ -96,7 +96,9 @@ export function getInFlight(placeId: string): Promise<{ filePath: string; attrib
export function setInFlight(placeId: string, promise: Promise<{ filePath: string; attribution: string | null } | null>): void {
inFlight.set(placeId, promise);
promise.finally(() => inFlight.delete(placeId));
promise
.finally(() => inFlight.delete(placeId))
.catch(() => { /* awaiter logs; this .catch only prevents unhandledRejection */ });
}
export function serveFilePath(placeId: string): string | null {
@@ -1235,7 +1235,7 @@ describe('getPlacePhoto (fetch stubbed)', () => {
// First call: get place details (with photos)
.mockResolvedValueOnce({
ok: true,
json: async () => ({
text: async () => JSON.stringify({
photos: [{ name: 'places/ChIJABC/photos/photo1', authorAttributions: [{ displayName: 'Photographer' }] }],
}),
})
@@ -1258,7 +1258,7 @@ describe('getPlacePhoto (fetch stubbed)', () => {
vi.stubGlobal('fetch', vi.fn().mockResolvedValue({
ok: false,
status: 403,
json: async () => ({ error: { message: 'Forbidden' } }),
text: async () => JSON.stringify({ error: { message: 'Forbidden' } }),
}));
const { getPlacePhoto } = await import('../../../src/services/mapsService');
const errId = `ChIJErr-${Date.now()}`;
@@ -1269,7 +1269,7 @@ describe('getPlacePhoto (fetch stubbed)', () => {
mockDbGet.mockReturnValueOnce({ maps_api_key: 'gkey' });
vi.stubGlobal('fetch', vi.fn().mockResolvedValue({
ok: true,
json: async () => ({ photos: [] }),
text: async () => JSON.stringify({ photos: [] }),
}));
const { getPlacePhoto } = await import('../../../src/services/mapsService');
const noPhotoId = `ChIJNone-${Date.now()}`;
@@ -1281,7 +1281,7 @@ describe('getPlacePhoto (fetch stubbed)', () => {
const fetchMock = vi.fn()
.mockResolvedValueOnce({
ok: true,
json: async () => ({
text: async () => JSON.stringify({
photos: [{ name: 'places/ChIJXYZ/photos/photo1', authorAttributions: [] }],
}),
})
@@ -1301,7 +1301,7 @@ describe('getPlacePhoto (fetch stubbed)', () => {
const fetchMock = vi.fn()
.mockResolvedValueOnce({
ok: true,
json: async () => ({
text: async () => JSON.stringify({
photos: [{ name: 'places/ChIJNoAttr/photos/photo1', authorAttributions: [] }],
}),
})