From 71e9dbfc968c68e245d57e947ecb8845106b2326 Mon Sep 17 00:00:00 2001 From: Claude Date: Wed, 12 Aug 2026 17:26:14 +0000 Subject: [PATCH] Separate App Store absence from transport failures Apple answers an unknown bundle ID with HTTP 200 and an empty result set, so lib/appStore.js was synthesising an "App not found (404)" error and the refresher was parsing that number back out of its own message. A genuine HTTP 404 from the transport landed in the same bucket, where it was exempted from the consecutive-failure cap as though six apps had left the store at once. Errors now carry the status code and Retry-After from the response, and inferred absence is flagged explicitly. Absence still bypasses the cap; a real 404 counts against it. The message keeps its historic shape because routes/index.js matches on it. A 403 or 429 also no longer records a failure against the app it interrupted. That app did nothing wrong, and the increment was costing it up to 30 days of backoff for a rate limit aimed at the client. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01158qZw5HJduNcc1srUvYP3 --- docs/app-store-metadata.md | 6 ++- lib/appStore.js | 23 +++++++++-- scripts/refresh-app-store-metadata.js | 36 +++++++++++++----- test/appStore.test.js | 11 ++++++ test/metadataJobs.test.js | 55 ++++++++++++++++++++++++++- 5 files changed, 116 insertions(+), 15 deletions(-) diff --git a/docs/app-store-metadata.md b/docs/app-store-metadata.md index c0f5661..1387bde 100644 --- a/docs/app-store-metadata.md +++ b/docs/app-store-metadata.md @@ -14,7 +14,11 @@ The service keeps three deliberately separate representations: `pnpm refresh-metadata` is the only scheduled path that deliberately requests Apple metadata. It defaults to 100 apps, a 30-day minimum age, and a 5-second delay between requests. The values can be changed with `--limit=`, `--min-age-days=`, and `--delay-ms=` or the corresponding `METADATA_REFRESH_*` environment variables. -Queued apps are selected first, followed by the oldest eligible refreshes. Failed rows back off exponentially from one day to a maximum of 30 days. Every attempt records `refresh_attempted_at`; failures increment `refresh_failures` and retain the last successful `details` and `fetched_at`. A 404 is stored as `app_not_found`, does not change `apps.status`, and does not consume the transport-failure cap. A 403 or 429 stops the run immediately, and five consecutive transport failures stop the run as well. +Queued apps are selected first, followed by the oldest eligible refreshes. Failed rows back off exponentially from one day to a maximum of 30 days. Every attempt records `refresh_attempted_at`; failures increment `refresh_failures` and retain the last successful `details` and `fetched_at`. + +Apple reports an unknown bundle ID as HTTP 200 with an empty result set, never as a 404, so `lib/appStore.js` infers absence and flags it. A flagged absence is stored as `app_not_found`, does not change `apps.status`, and does not consume the transport-failure cap. A genuine HTTP 404 is a routing or edge problem rather than a missing app, so it is stored with its own message and does count against that cap. Five consecutive transport failures stop the run. + +A 403 or 429 stops the run immediately and, because it describes this client rather than the app it interrupted, records no failure against that app. Apple's `Retry-After` is included in the reported stop reason when present. Search responses continue to populate the cache, behind a Cloudflare WAF challenge. A direct lookup only contacts Apple on a cache miss; its app insert and cache seed are committed in one transaction. Public `GET /analysis/:appId` never contacts Apple. diff --git a/lib/appStore.js b/lib/appStore.js index bd1aa02..054e116 100644 --- a/lib/appStore.js +++ b/lib/appStore.js @@ -1,6 +1,10 @@ const https = require('https'); const { isSameAppId, isValidAppId } = require('./appId'); +function appStoreError(message, properties) { + return Object.assign(new Error(message), properties); +} + function requestJson(url) { return new Promise((resolve, reject) => { const req = https.get(url, { timeout: 15000 }, (res) => { @@ -12,7 +16,10 @@ function requestJson(url) { }); res.on('end', () => { if (res.statusCode < 200 || res.statusCode >= 300) - return reject(new Error(`App Store request failed (${res.statusCode})`)); + return reject(appStoreError(`App Store request failed (${res.statusCode})`, { + statusCode: res.statusCode, + retryAfter: res.headers['retry-after'] || null + })); try { resolve(JSON.parse(body)); @@ -90,13 +97,21 @@ async function app({ appId, country }) { entity: 'software' }); const data = await requestJson(`https://itunes.apple.com/lookup?${params.toString()}`); + + // Apple answers an unknown bundle ID with HTTP 200 and an empty result set, + // never with a 404, so absence is something this function infers rather than + // something the transport reports. The `absent` flag carries that distinction + // to callers; the message keeps its historic "(404)" shape because + // routes/index.js matches on it. A real HTTP 404 is a different condition + // and arrives from requestJson carrying a statusCode instead. const result = data.results && data.results[0]; - if (!result) throw new Error('App not found (404)'); + if (!result) throw appStoreError('App not found (404)', { absent: true }); const normalized = normalize(result); - if (!isSameAppId(normalized.appId, appId)) throw new Error('App not found (404)'); + if (!isSameAppId(normalized.appId, appId)) + throw appStoreError('App not found (404)', { absent: true }); return normalized; } -module.exports = { search, app }; +module.exports = { search, app, appStoreError }; diff --git a/scripts/refresh-app-store-metadata.js b/scripts/refresh-app-store-metadata.js index ddfba5a..3208743 100644 --- a/scripts/refresh-app-store-metadata.js +++ b/scripts/refresh-app-store-metadata.js @@ -108,13 +108,20 @@ function appStoreStatus(error) { return match ? Number.parseInt(match[1], 10) : null; } +// Absence is inferred by lib/appStore.js from an empty 200 response, not +// reported by Apple as a 404. The message check keeps errors raised elsewhere +// (and older stored shapes) classified the same way. +function isAppAbsent(error) { + if (error && error.absent === true) return true; + return /App not found \(404\)/.test(String(error && error.message || error)); +} + function isRateLimitStop(error) { return [403, 429].includes(appStoreStatus(error)); } -function errorMessage(error) { - const status = appStoreStatus(error); - if (status === 404) return 'app_not_found'; +function errorMessage(error, absent = isAppAbsent(error)) { + if (absent) return 'app_not_found'; return String(error && error.message || error).slice(0, 2000); } @@ -196,16 +203,26 @@ async function refreshAppStoreMetadata(client, options = {}) { consecutiveFailures = 0; logger.log(`Refreshed ${row.appid}`); } catch (error) { - const message = errorMessage(error); - await recordFailure(client, row.appid, message); - failed++; - logger.warn(`Refresh failed for ${row.appid}: ${message}`); + const absent = isAppAbsent(error); + const message = errorMessage(error, absent); + // A 403 or 429 describes this client, not the app that happened to be + // next in the queue, so the run stops without recording a failure that + // would push an innocent app into exponential backoff. if (isRateLimitStop(error)) { - stoppedReason = `Apple request stop signal: ${message}`; + failed++; + const retryAfter = error && error.retryAfter; + stoppedReason = `Apple request stop signal: ${message}` + + (retryAfter ? ` (retry-after: ${retryAfter})` : ''); + logger.warn(`Refresh stopped at ${row.appid}: ${stoppedReason}`); break; } - if (appStoreStatus(error) === 404) { + + await recordFailure(client, row.appid, message); + failed++; + logger.warn(`Refresh failed for ${row.appid}: ${message}`); + + if (absent) { consecutiveFailures = 0; } else { consecutiveFailures++; @@ -261,6 +278,7 @@ module.exports = { parseArgs, buildRefreshSelectionQuery, appStoreStatus, + isAppAbsent, isRateLimitStop, refreshAppStoreMetadata, main diff --git a/test/appStore.test.js b/test/appStore.test.js index 5fe5bc4..e915257 100644 --- a/test/appStore.test.js +++ b/test/appStore.test.js @@ -14,6 +14,17 @@ test('lookup uses the UK App Store storefront', async () => { assert.equal(app.free, true); }); +test('an unknown bundle ID is reported as absence, not as a transport status', async () => { + const error = await store.app({ + appId: 'com.example.definitely.not.a.real.app.zzz999', + country: 'gb' + }).then(() => null, (err) => err); + + assert.ok(error, 'expected the lookup to reject'); + assert.equal(error.absent, true); + assert.equal(error.statusCode, undefined); +}); + test('search returns normalized UK App Store results', async () => { const results = await store.search({ term: 'whatsapp', diff --git a/test/metadataJobs.test.js b/test/metadataJobs.test.js index eea8e9f..727b2e2 100644 --- a/test/metadataJobs.test.js +++ b/test/metadataJobs.test.js @@ -90,9 +90,62 @@ test('refresh stops on a 429 and leaves remaining selections untouched', async ( assert.equal(result.refreshed, 1); assert.equal(result.failed, 1); assert.match(result.stoppedReason, /429/); + // The 429 describes the client, so the app it interrupted keeps a clean + // failure count instead of being pushed into exponential backoff. assert.equal( client.queries.filter(({ text }) => /refresh_failures = refresh_failures/.test(text)).length, - 1 + 0 + ); +}); + +test('a rate limit stop reports Apple\'s Retry-After when it sends one', async () => { + const client = refreshClient([{ appid: 'com.example.first', status: 'analysed' }]); + const result = await refresh.refreshAppStoreMetadata(client, { + delayMs: 0, + storeClient: { + async app() { + throw Object.assign(new Error('App Store request failed (429)'), { + statusCode: 429, + retryAfter: '120' + }); + } + }, + logger: silentLogger + }); + + assert.match(result.stoppedReason, /retry-after: 120/); +}); + +test('a real HTTP 404 counts against the transport failure cap', async () => { + const rows = Array.from({ length: 6 }, (_, index) => ({ + appid: `com.example.${index}`, + status: 'analysed' + })); + const client = refreshClient(rows); + let requests = 0; + const result = await refresh.refreshAppStoreMetadata(client, { + delayMs: 0, + storeClient: { + async app() { + requests++; + throw Object.assign(new Error('App Store request failed (404)'), { statusCode: 404 }); + } + }, + logger: silentLogger + }); + + // Apple signals a missing app with an empty 200, so a 404 from the transport + // is a routing or edge problem rather than six apps leaving the store. + assert.equal(requests, 5); + assert.equal(result.stoppedReason, '5 consecutive transport failures'); +}); + +test('absence is classified by flag as well as by legacy message shape', () => { + assert.equal(refresh.isAppAbsent(Object.assign(new Error('empty result'), { absent: true })), true); + assert.equal(refresh.isAppAbsent(new Error('App not found (404)')), true); + assert.equal( + refresh.isAppAbsent(Object.assign(new Error('App Store request failed (404)'), { statusCode: 404 })), + false ); });