fix(ra): let a failed disc hash explain itself in the log - #415
Merged
Conversation
A user reporting that no PlayStation 2 disc in their library matched sent an app.log whose only trace of the problem was the pass's own summary: RA re-match finished: 50 processed, 38 hashed, 28 matched, 12 skipped Every diagnostic behind that count was lost. Hashing runs under compute(), and a spawned isolate gets its own copy of the LoggerService singleton which never runs init() — so its output is ConsoleOutput alone. "CHD: could not open", "RA disc: no usable track", "RA disc: could not locate primary executable": all of it went to logcat, which a user does not have. It cannot simply open the file too, because two isolates holding a FileOutput on one path interleave writes and race the rotation rename. So the isolate hands its lines back instead. LoggerService gains a per-isolate capture — startCapture/takeCapture/replayCaptured — the hash isolate returns what it collected alongside the hash, and the main isolate replays it into the file-backed logger. Replaying goes through log() like any other line, so RedactingPrinter still strips credentials. A capture is capped at 64 lines: it exists to explain one failure, and an unbounded list in a background isolate is a leak waiting for a pathological image. The skip count now says why, per run and across the library: RA re-match finished: 50 processed, 38 hashed, 28 matched, 12 skipped (error 12) RA re-match: 12 ROM(s) parked (error 12) The second line is what finally gives getRaHashSkipCounts() a caller, and it matters because a pass over a settled library walks none of the parked ROMs — so on the run where a user notices the gap, the per-run line reads as a clean sweep. Verified against a container the reader cannot open: the two warnings that used to vanish now reach the log. AI-Assisted: Claude:Opus-5 Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
15 tasks
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description
A user reported that no PlayStation 2 disc in their library matched RetroAchievements and sent an
app.log. The only trace of the problem in it was the pass's own summary:Everything explaining those 12 was lost. Hashing runs under
compute(), and a spawned isolate gets its own copy of theLoggerServicesingleton which never runsinit()— so its output isConsoleOutputalone.CHD: could not open,RA disc: no usable track,RA disc: could not locate primary executableall went to logcat, which a user does not have. The isolate cannot simply open the log file too: two isolates holding aFileOutputon one path interleave writes and race the rotation rename, which is exactly why the secondary display's engine deliberately skipsinit().So the isolate hands its lines back instead.
LoggerServicegains a per-isolate capture (startCapture/takeCapture/replayCaptured),_generateHashForSystemIsolatereturns what it collected alongside the hash, and the main isolate replays it into the file-backed logger. Replaying goes throughlog()like any other line, soRedactingPrinterstill strips credentials before anything is written. A capture is capped at 64 lines — it exists to explain a single failure, and an unbounded list in a background isolate is a leak waiting for a pathological image.The skip count now says why, per run and across the library:
The second line is what finally gives
getRaHashSkipCounts()a caller, and it earns its place: a pass over a settled library walks none of the parked ROMs, so on the run where a user notices the gap the per-run line reads as a clean sweep.No behaviour changes — this only makes an existing failure legible. The underlying PS2 report is still open and gets its own PR once a log names the cause.
Steps to Verify
Preconditions: any platform, a library with at least one disc-based system, RetroAchievements signed in.
.chdof zeroes is enough (head -c 8192 /dev/zero > "Broken (USA).chd").app.login the user-data folder.... N skippedsummary. After it, the two warnings that used to reach logcat only are in the file:Tested on
flutter test, plus a real failing container throughRaDiscHash.compute)LoggerService.log, above every I/O path), and the isolate boundary it crosses is the same on every platform, so the risk of a device-only difference here is low. Happy to smoke it on the Thor before merge if preferred.Checklist
dart format .— no diffflutter analyze— clean (no errors or warnings)flutter test— all passing (1423)test/logger_isolate_capture_test.dart, 8 cases: per-level tagging, the error object, the 64-line cap, take-clears-capture, replay round-trip, malformed lines, and an unknown tag from a newer build