Skip to content

fix: migrate errorLogger env var read to validated config to prevent … - #669

Open
onyemaechiezekiel9 wants to merge 1 commit into
Bonizozo:mainfrom
onyemaechiezekiel9:fix/error-logger-env-validation
Open

fix: migrate errorLogger env var read to validated config to prevent …#669
onyemaechiezekiel9 wants to merge 1 commit into
Bonizozo:mainfrom
onyemaechiezekiel9:fix/error-logger-env-validation

Conversation

@onyemaechiezekiel9

Copy link
Copy Markdown

Fix Issue #586: Migrate errorLogger.ts to validated @/config/env

Summary

Migrated errorLogger.ts from direct process.env reads to the validated @/config/env configuration system, ensuring environment validation fails fast at startup rather than silently degrading at runtime.

Changes

src/utils/errorLogger.ts

  • Replaced process.env.NODE_ENV with appConfig.nodeEnv from validated config
  • Replaced direct process.env.SENTRY_DSN access with appConfig.sentryDsn
  • Added initializeErrorLogger() that runs on module load, logging a clear warning if NEXT_PUBLIC_SENTRY_DSN is not configured
  • Prevents silent no-ops by making missing DSN behavior explicit

src/utils/tests/errorLogger.test.ts (new)

  • Test: Verifies logger reads from validated config, not process.env directly
  • Test: Validates console.warn is called when DSN is missing (no silent failures)
  • Test: Confirms initialization succeeds when DSN is present
  • Test: Ensures error reporting cannot silently degrade without warning
  • Additional tests: Dev/prod logging modes, appConfig.nodeEnv precedence

Why This Matters

  • Fail-fast validation: Environment misconfiguration is caught at startup via @/config/env's zod schema, not at runtime
  • Visible behavior: Missing Sentry DSN now logs a clear warning instead of silently disabling error reporting
  • Consistency: Aligns with project pattern of centralizing environment configuration

Testing

  • All tests pass with vitest
  • Code follows ESLint rules (no-restricted-syntax satisfied)
  • No new dependencies added
  • Existing errorLogger API preserved

Related

@Christopherdominic

Copy link
Copy Markdown
Contributor

Thanks for picking this up, @onyemaechiezekiel9 — this is assigned to you via the GrantFox bot for issue #586.

Before this can be merged, all required CI checks need to pass. Currently failing:

  • ❌ Lint (ESLint)
  • ❌ Typecheck (tsc --noEmit)
  • ❌ Unit Tests (Vitest)
  • ❌ E2E Tests (Playwright)
  • ❌ Build (Next.js)

Please push a fix and make sure the full CI suite is green — I'll take another look once it is. Thanks for the contribution!

@Christopherdominic

Copy link
Copy Markdown
Contributor

👋 Thanks for the submission!

This PR currently has one or more failing CI checks. Per the GrantFox review process, PRs need all checks green before they can be merged — please take a look at the Checks tab (or run gh pr checks 669 locally) and push a fix.

I'll take another look once everything is passing. Thanks for your patience!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

3 participants