Skip to content

fix: restore verifyEmail error logging#2340

Draft
leno23 wants to merge 1 commit into
2anki:mainfrom
leno23:fix/2295-verify-email-log
Draft

fix: restore verifyEmail error logging#2340
leno23 wants to merge 1 commit into
2anki:mainfrom
leno23:fix/2295-verify-email-log

Conversation

@leno23
Copy link
Copy Markdown

@leno23 leno23 commented May 17, 2026

Summary

  • restore the verifyEmail catch-block error log before forwarding to Express error handling
  • add a regression test that asserts the underlying error is logged and passed to next(error)

Closes #2295

Tests

  • src/controllers/UsersControllers.ts and src/controllers/UsersControllers.test.ts parse successfully with the TypeScript parser
  • source-level proof confirmed the controller contains console.error('Email verification failed:', error) and the regression test asserts that log + next(error) path

Not run: pnpm test src/controllers/UsersControllers.test.ts because both git clone and GitHub archive download for a full checkout were denied in this environment. The patch is intentionally small and includes the focused Jest regression test for project CI to run.


View in Codesmith
Need help on this PR? Tag @codesmith with what you need.

  • Let Codesmith autofix CI failures and bot reviews

@sonarqubecloud
Copy link
Copy Markdown

Copy link
Copy Markdown
Contributor

@aalemayhu aalemayhu left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Welcome, and thanks for the focused fix — small PR, clear repro test, matches the parallel 'Magic link verification failed:' pattern at line 761. Nothing here blocks merge.

Verdict: approve (comment-only since I can't self-approve via tooling)

A missing log was making #2295 harder to debug; restoring it is a direct simplicity win.

Blocking

  • None.

Nits (optional, not asks)

  • src/controllers/UsersControllers.test.ts — the test asserts both calls happened but not the order. Since the regression is "log before forward", you can lock it in with expect(consoleError.mock.invocationCallOrder[0]).toBeLessThan(next.mock.invocationCallOrder[0]). The source has them on consecutive lines so it's implicit today, but the assertion would catch a future reorder.
  • src/controllers/UsersControllers.test.ts — the try/finally + mockRestore is correct but slightly heavier than needed; verifyEmail catches its own errors, so a trailing consoleError.mockRestore() would do. Matter of taste.

When you're ready, flip from draft with gh pr ready 2340 and I'll merge.

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Restore error log in verifyEmail before next(error)

2 participants