Skip to content

chore(auth): drop dead attachOk guard in resolveAuth#256

Merged
CryptoJones merged 1 commit into
masterfrom
docs/auth-resolveauth-drop-stale-500-comment
May 19, 2026
Merged

chore(auth): drop dead attachOk guard in resolveAuth#256
CryptoJones merged 1 commit into
masterfrom
docs/auth-resolveauth-drop-stale-500-comment

Conversation

@CryptoJones
Copy link
Copy Markdown
Owner

Summary

Cleanup follow-up to #162, which removed the outer try/catch around isMaster + getCompanyId in attachAuth.

Since that change, attachAuth no longer has any 500-emitting path — its callees swallow DB-infrastructure errors into sentinels (false / -1) and always invoke next(). The attachOk boolean in resolveAuth was therefore guaranteed true after the awaited Promise resolved, and the comment "attachAuth already sent a 500" misled readers into thinking there was still a divergent error path.

Drop the dead conditional and its stale comment. Behavior unchanged.

Test plan

  • npm run lint && npm test — 688 passing, no regressions
  • Unit + integration auth tests still cover both attach paths and the strict 403 in requireAuth

Proudly Made in Nebraska. Go Big Red! 🌽 https://xkcd.com/2347/

Cleanup follow-up to #162, which removed the outer try/catch in
attachAuth. Since attachAuth no longer has any 500-emitting path (its
internal callees swallow infrastructure errors into sentinels and
always invoke next()), the `attachOk` boolean was guaranteed true
after the awaited Promise resolved.

The dead `if (!attachOk) return;` line and its stale
"attachAuth already sent a 500" comment misled readers into thinking
resolveAuth still had a divergent error path. Drop both. Behavior
unchanged.

All 688 tests still pass.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
@CryptoJones CryptoJones merged commit d751332 into master May 19, 2026
3 checks passed
@CryptoJones CryptoJones deleted the docs/auth-resolveauth-drop-stale-500-comment branch May 19, 2026 12:59
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.

1 participant