fix: debug log file mismatch and missing http warning entries in debug log#182
Open
Vinyl-Davyl wants to merge 2 commits intoNetflix:masterfrom
Conversation
Author
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.
Requirements for a pull request
Description of the Change
Addresses Issue #181
Fixes two defects in the debug logging infrastructure across two small utility files:
1. src/utils/debugdb.ts — Malformed log filename
A double-closing-brace typo (
}}) in the getLogs() template literal causedevery downloaded debug log to have a spurious
}appended to its filename(e.g.
logs-2025-01-01T00:00:00.000Z}.txt). This is corrected to produce theintended
logs-<ISO timestamp>.txt.2. src/utils/errorlogger.ts — HTTP warnings not persisted to debug log
logWarning is called by useResource on every HTTP error, but previously only
invoked
console.warn. It did not call setLogItem, so warnings were invisiblein downloaded crash reports. This change routes all warnings into the IndexedDB
debug log and restores version environment prefixing on the console output
(the getVersionInfo import was already present but commented out).
Alternate Designs
An alternative would be to update every logWarning call site to also call
setLogItem directly. This was ruled out because it would scatter logging
boilerplate across multiple files and violate the single-responsibility principle
of the
errorloggermodule. Centralising the behaviour in one place is cleanerand ensures any future callers automatically benefit.
Possible Drawbacks
debugdb. If debug logging is notinitialised (i.e. startLogging() has not been called), setLogItem is a
no-op, so there is no runtime error — warnings are simply not persisted, which
is the existing behaviour.
occurs in the debug-mode code path.
Verification Process
}character..txtfile and confirmWARN HTTP error id: not-foundis present.Release Notes
Fixed a bug where downloaded debug log files had a malformed filename ending in
}. HTTP warning messages are now also captured in the debug log, makingdownloaded crash reports more complete. Fixes #181