chore(logging): strip debug console.log noise + enforce no-console lint rule - #189
Merged
Conversation
…nt rule
The codebase had ~109 console.log/info/debug calls scattered through src/,
mostly QRZ/LoTW sync debug scaffolding and ADIF import batch tracers. None
of them shipped useful runtime info — they were development breadcrumbs
that never got cleaned up. Strip them and add an ESLint rule so they
can't quietly come back.
Heavy hitters cleaned:
- src/app/api/contacts/qrz-sync/route.ts: 28 per-contact debug traces
- src/lib/qrz.ts: 20 HTTP request/response traces
- src/app/api/contacts/qrz-download/route.ts: 8
- src/app/api/install/finalize/route.ts: 7 (one converted to console.warn)
- src/app/api/cron/lotw-{download,upload}/route.ts: 12 combined (auth/header
debug + per-station progress)
- src/app/api/adif/import/route.ts + src/components/AutoImportADIF.tsx:
13 batch-progress traces
- src/app/api/lotw/download{,-contact}/route.ts: 12 credential-path traces
- src/app/api/install/migrate-schema/route.ts: 4
- Smaller cleanups in UserContext, search/page, dxpeditions, settings
Kept:
- console.error in genuine error paths (unchanged policy)
- console.warn for real warnings (e.g. fallback paths in Propagation,
install/database, install/finalize, install/migrate-schema)
- The two intentional console.log calls in src/lib/logger.ts (with
eslint-disable-next-line annotations, since the logger module is
precisely what the rest of the codebase should use going forward)
ESLint:
- Added `no-console` rule with `allow: ["warn", "error"]` at error level
- Exempted scripts/**/*.{js,mjs} (CLI utilities legitimately use console)
- Exempted tests/**/*.{ts,js} (diagnostic output in test failures)
Also picks up the same one-line null-guard fix from PR #188 in
tests/database-integration.spec.ts so this branch's `tsc --noEmit` runs
clean independent of merge order.
Lint: 0 errors, 52 warnings (unchanged baseline; React hooks warnings
tracked separately for PR 3).
Build: succeeds.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
6 tasks
patrickrb
added a commit
that referenced
this pull request
May 11, 2026
Starting from 52 warnings. End state: 0 errors, 0 warnings. Mix of substantive fixes and one documented rule downgrade. Each category: Hoisting / stale-closure (17 warnings) — wrapped fetcher functions in useCallback and moved their declarations above the useEffects that call them. Adds proper dep arrays. Affected pages: - adif/page.tsx (fetchStations) - admin/storage/page.tsx (fetchConfigs) - admin/users/page.tsx (fetchUsers) - awards/dxcc/page.tsx (fetchDXCCSummary) - awards/was/page.tsx (loadStations) - new-contact/page.tsx (fetchStations, fetchCurrentUser) - stations/[id]/edit/page.tsx (fetchStation + 3 siblings) - stations/new/page.tsx (fetchDxccEntities, fetchStatesProvinces) - stations/page.tsx (fetchStations, fetchStationStats) - stats/page.tsx (fetchStations) - search/page.tsx (performSearch, debouncedSearch — moved above the useEffect that triggers it) Memoization warning (1) — search/page.tsx `debouncedSearch` was caught by react-hooks/preserve-manual-memoization because it depended on `searchTimeout` state and called `setSearchTimeout`, recreating itself on every tick. Replaced the state with a useRef so the callback's identity is stable. This is the React-19-compiler-recommended pattern for timer state. set-state-in-effect (33 warnings) — disabled. These all fired on the standard "fetch data on mount → setState with the result" pattern, which is normal React data-loading. Per-line suppression would add 33 comments across the codebase, noisier than the warning itself. eslint.config.mjs carries a comment explaining the decision and pointing at the path to re-enable (adopt SWR/TanStack Query, which obviates the pattern). Unused eslint-disable in storage.ts (1) — the comments suppressed no-unused-vars for parameters that already had `_` prefix. Added `argsIgnorePattern: "^_"` to the project ESLint config (standard JS/TS convention for "intentionally unused"), then dropped the now-redundant disable comments. Two follow-on `_mimeType` warnings disappeared too. Also picks up the same one-line null-guard in tests/database-integration .spec.ts as PRs #188/#189, so this branch's `tsc --noEmit` is clean independent of merge order. Verification: - npm run lint → 0 errors, 0 warnings - npx tsc --noEmit → clean - npm run build → succeeds Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
5 tasks
patrickrb
added a commit
that referenced
this pull request
May 11, 2026
Two concrete issues in error response bodies:
1. Awards routes (4 files) leaked raw error.message to clients in their
typed `{ success: false, error }` responses. A catch over a DB query
can surface column names, constraint names, or stack frames in the
response body — useful in dev, a soft information disclosure in prod.
Fixed: the real error stays in console.error for server-side
observability; the client gets 'Internal server error'.
2. LoTW download routes (2 files) returned a `debug: {...}` object
alongside credential-error responses, exposing internal state like
`has_username: bool`, `credential_source`, and station identifiers
to a 400 response. Leftover from the QRZ sync debug-strip pass
(#189) — the frontend never consumed these fields. Stripped.
The discriminated-union `{ success, data, error }` shape used by
`/api/awards/*` and `/api/cloudlog/*` stays — they're documented
contracts with multiple consumers (frontend checks `data.success`;
cloudlog is a public Cloudlog-compatible API). The `details:` field
on install/cron error responses also stays — it's admin/diagnostic
flow and not reachable by untrusted callers.
CLAUDE.md updated:
- Documents both acceptable response patterns and when each applies
- Adds explicit "don't leak raw error.message" rule with an example
- Notes the cloudlog/* external API contract
- Refreshed the Logging section now that the no-console rule (from
#189) has landed; points new code at src/lib/logger.ts
Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
5 tasks
patrickrb
added a commit
that referenced
this pull request
May 11, 2026
Worked through every TODO in src/, sorted by what was actually actionable:
Implemented (4 → 0):
- src/app/api/awards/was/summary/route.ts — completed_awards now reflects
the WAS Basic award (all 50 US states confirmed → 1 else 0). The
/awards page already displays this prominently as "Awards Earned";
it was always showing 0 before. total_was_awards = 1 (we track the
Basic tier only; extensible when we add 5-Band/Triple Play/etc.).
- src/app/api/awards/dxcc/summary/route.ts — same pattern. DXCC Basic
requires 100+ confirmed entities. total_dxcc_awards = 1 (Basic tier;
extensible when we add 150/200/.../350, by-band, by-mode).
Fixed an honest-output issue (2 → 0):
- src/app/api/cloudlog/qso/route.ts:187 and modes/route.ts:137 hardcoded
X-RateLimit-Remaining: 999. The cloudlog index docs advertise the
rate-limit headers as part of the public API contract, but enforcement
isn't implemented. Sending "999" to consumers is a lie. Now reports
Remaining == Limit (effectively "no enforcement, unlimited within the
documented limit") with a comment pointing at the path forward.
Deleted as redundant (2 → 0):
- src/app/admin/page.tsx:132,144 — "TODO: Implement bulk LoTW
{upload,download}" sat one line above setSyncMessage('coming soon!').
The UI message already documents the state; the TODO was clutter.
Deleted as dead code (1 → 0):
- src/app/search/page.tsx:624 — TODO marked a `messages` array that got
built up but never displayed. It was a leftover from a console.log
removed in #189. The actual user feedback (table refresh with new
sync status indicators) still works. Removed the dead build + TODO.
Kept (1 → 1):
- src/lib/storage.ts:318 — "TODO: Implement AWS S3 deletion". The AWS
S3 storage backend isn't implemented at all (uploadToS3 also fails
gracefully); this TODO accurately documents the unimplemented branch.
Removing it doesn't help; implementing AWS S3 is its own future PR.
Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
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
The codebase had ~109
console.log/info/debugcalls scattered throughsrc/, mostly QRZ/LoTW sync debug scaffolding and ADIF import batch tracers. They were development breadcrumbs that never got cleaned up — none shipped useful runtime info. This PR strips them and adds an ESLint rule so they can't quietly come back.Heavy hitters cleaned:
src/app/api/contacts/qrz-sync/route.ts— 28 per-contact debug tracessrc/lib/qrz.ts— 20 HTTP request/response tracessrc/app/api/contacts/qrz-download/route.ts— 8src/app/api/install/finalize/route.ts— 7 (one converted toconsole.warnfor real)src/app/api/cron/lotw-{download,upload}/route.ts— 12 combined (auth/header debug + per-station progress)src/app/api/adif/import/route.ts+src/components/AutoImportADIF.tsx— 13 batch-progress tracessrc/app/api/lotw/download{,-contact}/route.ts— 12 credential-path tracessrc/app/api/install/migrate-schema/route.ts— 4UserContext,search/page,dxpeditionsKept:
console.errorin genuine error paths (unchanged policy)console.warnfor real warnings (e.g. fallback paths inPropagation, install routes)console.logcalls insrc/lib/logger.ts(witheslint-disable-next-line— the logger module is precisely what the rest of the codebase should use going forward)ESLint:
no-consolerule withallow: ['warn', 'error']at error levelscripts/**/*.{js,mjs}(CLI utilities legitimately use console)tests/**/*.{ts,js}(diagnostic output in test failures)Notes for review
tests/database-integration.spec.tsso this branch'stsc --noEmitruns clean independently of merge order. When chore(api): standardize JSON field casing on snake_case + add CLAUDE.md #188 merges first, this will resolve as the same line.console.loginsrc/— phasing out, lint rule coming." This is that follow-up.Test plan
npm run lint— 0 errors, 52 warnings (unchanged baseline)npx tsc --noEmit— cleannpm run build— succeeds🤖 Generated with Claude Code