feat: error log admin diagnostics + multi-role dean login fix#397
Merged
Conversation
Two related changes bundled together — the dean login 500 was the catalyst for the diagnostic page, so both ship in the same PR. ## Error log admin diagnostics Captures unhandled 5xx exceptions so they're visible on the admin console instead of vanishing into a generic 500 response. - new `error_log` table (entity + migration) — never soft-deleted, indexed on status_code / path / user_id / error_name / acknowledged_at / occurred_at for filter performance - `SystemErrorsModule` (`src/modules/system-errors/`) mirrors `AuditModule` shape: `ErrorLogService.Emit()` async-enqueues via BullMQ, `ErrorLogProcessor` persists, `ErrorLogQueryService` handles paginated reads + acknowledge flow, `ErrorLogController` exposes `/error-logs` endpoints behind a `@UseJwtGuard(SUPER_ADMIN)` - `ErrorCaptureFilter` global exception filter: catches everything, only persists ≥500, redacts sensitive keys (`password`/`token`/`refreshToken`/`authorization` etc.) and caps payload depth/breadth/string length, then defers to `BaseExceptionFilter` so the wire response is byte-identical to today - `ErrorLogCleanupJob` daily at 04:00 UTC — 90-day retention via `nativeDelete` on `occurred_at` - 12 new unit tests covering sanitizer behaviour and filter capture paths (4xx skipped, 5xx captured, non-http hosts skipped, capture failures swallowed without re-throwing) ## Multi-role dean login fix (ucmn-t-67092) Adds defensive guards to `MoodleUserHydrationService.resolveInstitutionalRoles` so users with both DEAN and CHAIRPERSON institutional roles can log in when the populated `moodleCategory` relation is null (soft-delete filter or drift after Moodle restructure). Seven previously-unguarded `ir.moodleCategory.moodleCategoryId` dereferences now optional-chain + filter; orphaned auto rows are now removed instead of crashing. Regression tests cover both DEAN-orphan and CHAIRPERSON-orphan shapes. Single-role deans skip the (DEAN ∧ CHAIRPERSON) cleanup branch, which is why only the intersection user tripped the TypeError. ## Migration scope (verified safe on staging) This migration also adds `analysis_pipeline_trigger_check` — `@Enum(() => PipelineTrigger)` is declared on the entity but the CHECK was missing from the DB. Verified: 0 violating rows, no name collision. Does NOT include the CLI-suggested `drop index uq_analysis_pipeline_active_scope`. That's the FAC-132 partial unique index — MikroORM decorators can't represent it (see new gotcha entry in `src/entities/CLAUDE.md`) and dropping it would reintroduce the duplicate-active-pipeline bug it fixed. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
4 tasks
y4nder
added a commit
that referenced
this pull request
May 12, 2026
…398) Two related changes bundled together — the dean login 500 was the catalyst for the diagnostic page, so both ship in the same PR. ## Error log admin diagnostics Captures unhandled 5xx exceptions so they're visible on the admin console instead of vanishing into a generic 500 response. - new `error_log` table (entity + migration) — never soft-deleted, indexed on status_code / path / user_id / error_name / acknowledged_at / occurred_at for filter performance - `SystemErrorsModule` (`src/modules/system-errors/`) mirrors `AuditModule` shape: `ErrorLogService.Emit()` async-enqueues via BullMQ, `ErrorLogProcessor` persists, `ErrorLogQueryService` handles paginated reads + acknowledge flow, `ErrorLogController` exposes `/error-logs` endpoints behind a `@UseJwtGuard(SUPER_ADMIN)` - `ErrorCaptureFilter` global exception filter: catches everything, only persists ≥500, redacts sensitive keys (`password`/`token`/`refreshToken`/`authorization` etc.) and caps payload depth/breadth/string length, then defers to `BaseExceptionFilter` so the wire response is byte-identical to today - `ErrorLogCleanupJob` daily at 04:00 UTC — 90-day retention via `nativeDelete` on `occurred_at` - 12 new unit tests covering sanitizer behaviour and filter capture paths (4xx skipped, 5xx captured, non-http hosts skipped, capture failures swallowed without re-throwing) ## Multi-role dean login fix (ucmn-t-67092) Adds defensive guards to `MoodleUserHydrationService.resolveInstitutionalRoles` so users with both DEAN and CHAIRPERSON institutional roles can log in when the populated `moodleCategory` relation is null (soft-delete filter or drift after Moodle restructure). Seven previously-unguarded `ir.moodleCategory.moodleCategoryId` dereferences now optional-chain + filter; orphaned auto rows are now removed instead of crashing. Regression tests cover both DEAN-orphan and CHAIRPERSON-orphan shapes. Single-role deans skip the (DEAN ∧ CHAIRPERSON) cleanup branch, which is why only the intersection user tripped the TypeError. ## Migration scope (verified safe on staging) This migration also adds `analysis_pipeline_trigger_check` — `@Enum(() => PipelineTrigger)` is declared on the entity but the CHECK was missing from the DB. Verified: 0 violating rows, no name collision. Does NOT include the CLI-suggested `drop index uq_analysis_pipeline_active_scope`. That's the FAC-132 partial unique index — MikroORM decorators can't represent it (see new gotcha entry in `src/entities/CLAUDE.md`) and dropping it would reintroduce the duplicate-active-pipeline bug it fixed. Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
5 tasks
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
Two related changes bundled together — the multi-role dean login 500 was the catalyst for the diagnostic page, so both ship together.
error_logtable, BullMQ-async global exception filter that captures unhandled 5xx with sanitized request bodies, admin/error-logsendpoints, 90-day cleanup cron. Surfaces in the admin.faculytics errors page so the "Internal server error" wall in staging is no longer opaque.ucmn-t-67092): guards against nullmoodleCategoryafter populate inresolveInstitutionalRoles. Single-role deans never hit the (DEAN ∧ CHAIRPERSON) cleanup branch — only the intersection user tripped theTypeError.analysis_pipeline_trigger_checkCHECK constraint (verified safe on staging: 0 violating rows, no name collision). Intentionally does not dropuq_analysis_pipeline_active_scope— that's the FAC-132 partial unique index the CLI can't model. See new gotcha insrc/entities/CLAUDE.md.Frontend pairing
The admin diagnostics page itself lives in admin.faculytics PR.
Test plan
npm run test— 1148 passingnpm run lint(0 errors in changed files; pre-existing warnings unrelated)npm run start:devregistersSystemErrorsModuleand mountsErrorLogController {/api/error-logs}npx mikro-orm migration:up— verified non-destructive (createserror_log+ addsanalysis_pipeline_trigger_checkconstraint only)ucmn-t-67092BEFORE merging this fix) and confirm it appears in the admin errors pageucmn-t-67092AFTER merging — should succeed (defensive guards in place)🤖 Generated with Claude Code