fix: three bug fixes — null guard crashes, silent catches, NaN metadata#1270
Draft
cursor[bot] wants to merge 4 commits into
Draft
fix: three bug fixes — null guard crashes, silent catches, NaN metadata#1270cursor[bot] wants to merge 4 commits into
cursor[bot] wants to merge 4 commits into
Conversation
formatIssueCell() passed issue.title directly to escapeMarkdownInline() which calls .replace() — crashes with TypeError when the API returns a null title. The same file already guards this in formatIssueDetails() (issue.title ?? ""). Similarly, formatAuthStatus() passed org.name without a fallback, while formatOrgDetails() in the same file already uses org.name || "(unnamed)". Add matching null guards to both call sites. Co-authored-by: Miguel Betegón <miguelbetegongarcia@gmail.com>
dashboard/create.ts: enrichTargetProjectIds() silently dropped projects when getProject() failed — the dashboard could be created without the expected project scope and no diagnostic trail. Add log.debug() so failures are visible with --verbose. response-cache.ts: invalidateCachedResponsesMatching() had three silent catch blocks (parse error, unlink race, outer IO) that swallowed all errors. Cache corruption or permission issues were invisible. Add log.debug() calls matching the project's documented catch-block policy. Co-authored-by: Miguel Betegón <miguelbetegongarcia@gmail.com>
Number() on a non-numeric string (e.g. from DB corruption or manual edits) returns NaN. In version-check.ts, NaN lastChecked causes Date.now() - NaN → NaN, making all comparisons false and permanently disabling version checks with no visible indication. Similarly, NaN lastNotified permanently suppresses update notifications. In install-info.ts, NaN recordedAt would break any downstream timestamp comparisons. Add Number.isNaN() guards that fall back to null/0, restoring the 'never checked' / 'epoch' state so the next check runs normally. Co-authored-by: Miguel Betegón <miguelbetegongarcia@gmail.com>
Co-authored-by: Miguel Betegón <miguelbetegongarcia@gmail.com>
Contributor
|
Contributor
Codecov Results 📊❌ Patch coverage is 77.78%. Project has 5481 uncovered lines. Files with missing lines (3)
Coverage diff@@ Coverage Diff @@
## main #PR +/-##
==========================================
+ Coverage 81.67% 81.67% —%
==========================================
Files 426 426 —
Lines 29896 29903 +7
Branches 19420 19434 +14
==========================================
+ Hits 24416 24422 +6
- Misses 5480 5481 +1
- Partials 2035 2035 —Generated by Codecov Action |
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
1 participant
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.
Summary
Three independent defensive-coding fixes found via codebase analysis.
1.
formatIssueCellandformatAuthStatuscrash on null API fieldsRoot cause:
escapeMarkdownInline(issue.title)informatIssueCell()calls.replace()— crashes withTypeErrorwhen the API returns a null title. Same pattern withorg.namein auth status display.Reproduction:
sentry issue listwhen any issue in the response hastitle: null(can happen with deleted/malformed issues).sentry auth statuswhen any org hasname: null.Fix: Add
?? ""and|| "(unnamed)"guards matching existing patterns in the same file (formatIssueDetailsat line 806,formatOrgDetailsat line 1486).Files:
src/lib/formatters/human.ts2. Silent catch blocks in
dashboard/create.tsandresponse-cache.tsRoot cause:
enrichTargetProjectIds()indashboard/create.tssilently drops projects whengetProject()fails — the dashboard is created without the expected project scope with no diagnostic trail.invalidateCachedResponsesMatching()inresponse-cache.tshas three catch blocks that swallow all errors silently, making cache corruption invisible.Reproduction:
sentry dashboard createwith auto-detect when a project API call fails (network glitch, permissions). Cache invalidation failures are invisible in debug logs.Fix: Add
log.debug()calls to all silent catches, matching the project's documented catch-block policy (AGENTS.md).Files:
src/commands/dashboard/create.ts,src/lib/response-cache.ts3. NaN timestamps from corrupted numeric metadata
Root cause:
Number()on a non-numeric string returnsNaN. Inversion-check.ts,NaNlastCheckedcausesDate.now() - NaN → NaN, making all comparisons false and permanently disabling version checks with no visible indication. Same pattern ininstall-info.tsforrecordedAt.Reproduction: Database corruption or manual edits that store non-numeric strings in the metadata table for version check timestamps.
Fix: Add
Number.isNaN()guards that fall back tonull/0, restoring the "never checked" / "epoch" state so the next check runs normally.Files:
src/lib/db/version-check.ts,src/lib/db/install-info.tsTest verification
invalidateCachedResponsesMatching)