refactor(appview): extract shared error handling, ban check middleware, and ForumAgent helper (#52)
* refactor(appview): extract shared error handling, ban check middleware, and ForumAgent helper
Eliminates ~500 lines of duplicated boilerplate across all route handlers by
extracting three reusable patterns:
- handleReadError/handleWriteError/handleSecurityCheckError: centralized error
classification (programming errors re-thrown, network→503, database→503,
unexpected→500) replacing ~40 inline try-catch blocks
- safeParseJsonBody: replaces 9 identical JSON parsing blocks
- requireNotBanned middleware: replaces duplicate ban-check-with-error-handling
in topics.ts and posts.ts
- getForumAgentOrError: replaces 6 identical ForumAgent availability checks
in mod.ts and admin.ts
* fix(review): address PR #52 review feedback on shared error handling refactor
C1: Update test assertions to match centralized error messages
C2: Add isProgrammingError re-throw to handleReadError
C3: Delete parseJsonBody — false JSDoc, deleted in favor of safeParseJsonBody
I1: Add 503 classification (isNetworkError + isDatabaseError) to handleReadError
I2: Add isNetworkError check to handleSecurityCheckError
I3: Restore original middleware ordering — ban check before permission check
M1: Add unit tests for route-errors.ts and require-not-banned.ts
Also: Remove redundant isProgrammingError guards in admin.ts
* fix(review): re-throw programming errors in fail-open GET topic catch blocks
Ban check, hidden-posts check, and mod-status check in GET /api/topics/:id
are fail-open (continue on transient DB failure). But without an
isProgrammingError guard, a TypeError from a code bug would silently skip
the safety check — banned users' content visible, hidden posts visible, or
locked topics accepting replies. isProgrammingError was already imported.
* refactor(appview): remove duplicate requireNotBanned from permissions.ts
PR 51 added requireNotBanned to permissions.ts; PR 52 introduced a
dedicated require-not-banned.ts with the improved implementation that
uses handleSecurityCheckError (proper network+DB classification, dynamic
operation name, correct 500 message). All callers already import from
require-not-banned.ts. Remove the stale copy and its now-unused imports
(getActiveBans, isProgrammingError, isDatabaseError).
---------
Co-authored-by: Claude <noreply@anthropic.com>