Skip to content

Commit dd2d935

Browse files
fix: address Copilot review — proxy fetchHTML, doc comments, test assertions
- worker.js: fetchHTML now throws on 5xx/network errors so transient upstream failures set X-Proxy-Status: error and are not KV-cached; 4xx still returns null (legitimate "not found") - PVSettingsModel: correct useCheatProxy doc comment — fallback only on proxy error/unreachable, not on empty result with ok status - GameHackingOrgLookupTests: assert URLProtocol.registerClass return value so test fails fast instead of silently hitting real network - cheat-proxy README: document X-Proxy-Status header semantics and correct fallback behaviour description Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
1 parent a385df7 commit dd2d935

4 files changed

Lines changed: 26 additions & 20 deletions

File tree

PVLibrary/Tests/PVLibraryTests/GameHackingOrgLookupTests.swift

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -120,7 +120,7 @@ final class GameHackingOrgLookupTests: XCTestCase {
120120
// MARK: - Proxy Path (URLProtocol stubs)
121121

122122
func testSearchCheats_proxyReturnsResults() async {
123-
URLProtocol.registerClass(ProxyCannedProtocol.self)
123+
XCTAssertTrue(URLProtocol.registerClass(ProxyCannedProtocol.self), "URLProtocol registration failed — test may hit real network")
124124
defer { URLProtocol.unregisterClass(ProxyCannedProtocol.self) }
125125

126126
let json = #"[{"name":"Infinite Lives","code":"DEADBEEF00000001","category":"General"}]"#
@@ -157,8 +157,8 @@ final class GameHackingOrgLookupTests: XCTestCase {
157157
// Proxy returns [] with X-Proxy-Status: ok — meaning "upstream confirmed no cheats".
158158
// searchCheats should trust this and NOT fall back to direct scraping.
159159
// DirectScrapeBlockerProtocol is registered to ensure no gamehacking.org request is made.
160-
URLProtocol.registerClass(ProxyCannedProtocol.self)
161-
URLProtocol.registerClass(DirectScrapeBlockerProtocol.self)
160+
XCTAssertTrue(URLProtocol.registerClass(ProxyCannedProtocol.self), "URLProtocol registration failed — test may hit real network")
161+
XCTAssertTrue(URLProtocol.registerClass(DirectScrapeBlockerProtocol.self), "URLProtocol registration failed — test may hit real network")
162162
defer {
163163
URLProtocol.unregisterClass(ProxyCannedProtocol.self)
164164
URLProtocol.unregisterClass(DirectScrapeBlockerProtocol.self)
@@ -196,8 +196,8 @@ final class GameHackingOrgLookupTests: XCTestCase {
196196
// Proxy is disabled — only the direct scrape path runs.
197197
// DirectScrapeBlockerProtocol intercepts gamehacking.org requests so no real
198198
// network call is made; it returns empty HTML so the scrape yields no results.
199-
URLProtocol.registerClass(ProxyCannedProtocol.self)
200-
URLProtocol.registerClass(DirectScrapeBlockerProtocol.self)
199+
XCTAssertTrue(URLProtocol.registerClass(ProxyCannedProtocol.self), "URLProtocol registration failed — test may hit real network")
200+
XCTAssertTrue(URLProtocol.registerClass(DirectScrapeBlockerProtocol.self), "URLProtocol registration failed — test may hit real network")
201201
defer {
202202
URLProtocol.unregisterClass(ProxyCannedProtocol.self)
203203
URLProtocol.unregisterClass(DirectScrapeBlockerProtocol.self)

PVSettings/Sources/PVSettings/Settings/Model/PVSettingsModel.swift

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -312,8 +312,10 @@ public extension Defaults.Keys {
312312
// MARK: Cheats
313313
public extension Defaults.Keys {
314314
/// When `true`, the app first queries the cheat proxy endpoint for GameHacking.org
315-
/// cheats instead of scraping the site directly. Falls back to direct scraping
316-
/// if the proxy returns an empty result or is unreachable.
315+
/// cheats instead of scraping the site directly. Falls back to direct scraping only
316+
/// if the proxy is unreachable, returns a non-2xx status, or returns an empty result
317+
/// without confirming success via `X-Proxy-Status: ok`. An empty result with
318+
/// `X-Proxy-Status: ok` is treated as "no cheats found" and skips the fallback.
317319
static let useCheatProxy = Key<Bool>("useCheatProxy", default: true)
318320

319321
/// Base URL of the deployed Provenance cheat proxy worker.

Scripts/cheat-proxy/README.md

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -24,8 +24,12 @@ GET /cheats?title=<game title>&system=<system slug>
2424
Returns an empty array `[]` when no cheats are found or when the request is invalid
2525
(e.g. missing `title`, unknown path). The response is always HTTP 200 with a JSON array
2626
so clients can safely decode without checking the status code. Invalid requests also
27-
include an `X-Validation-Error` header with a short description. The caller should fall
28-
back to direct scraping on an empty result.
27+
include an `X-Validation-Error` header with a short description.
28+
29+
Responses include an `X-Proxy-Status` header (`ok` or `error`) indicating whether the
30+
proxy successfully contacted GameHacking.org. Callers should treat an empty array with
31+
`X-Proxy-Status: ok` as "confirmed no cheats found" and skip the direct-scrape fallback;
32+
fall back only when the proxy request fails or `X-Proxy-Status` is missing or `error`.
2933

3034
## Health check
3135

Scripts/cheat-proxy/worker.js

Lines changed: 11 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -214,18 +214,18 @@ function buildSearchURL(title, systemSlug) {
214214
}
215215

216216
async function fetchHTML(url) {
217-
try {
218-
const resp = await fetch(url, {
219-
headers: {
220-
"User-Agent": USER_AGENT,
221-
"Accept": "text/html,application/xhtml+xml",
222-
},
223-
});
224-
if (!resp.ok) return null;
225-
return await resp.text();
226-
} catch {
227-
return null;
217+
const resp = await fetch(url, {
218+
headers: {
219+
"User-Agent": USER_AGENT,
220+
"Accept": "text/html,application/xhtml+xml",
221+
},
222+
});
223+
if (resp.status >= 500) {
224+
// Server error — treat as transient failure so it is not KV-cached
225+
throw new Error(`Upstream HTTP ${resp.status} from ${url}`);
228226
}
227+
if (!resp.ok) return null; // 4xx → legitimate "not found"
228+
return await resp.text();
229229
}
230230

231231
// ─── Search result parsing ────────────────────────────────────────────────────

0 commit comments

Comments
 (0)