fix(run): skip unqueryable tables and record sanitized error detail #33

Merged
KofTwentyTwo merged 1 commits from feature/backups-error-detail-unqueryable into develop 2026-10-16 16:51:15 +00:00
KofTwentyTwo commented 2026-10-16 16:41:44 +00:00 (Migrated from github.com)

Why

kof22 health reports backups DOWN. Prod Loki logs show backups_run 5 and 6 (2026-10-16) both failed about 7 s in:

java.lang.IllegalStateException: Query is not implemented in this module: PreviewDownloadBackend
  at QBackendModuleInterface.throwNotImplemented(QBackendModuleInterface.java:254)
  at QBackendModuleInterface.getQueryInterface(QBackendModuleInterface.java:126)
  at QueryAction.executeInAuthorizedScope(QueryAction.java:259)
  at KeysetPager.fetchNextPage(KeysetPager.java:427) / forEachPage(259)
  at com.kingsrook.qbits.backups.run.TableReader$Keyset.read(TableReader.java:92)
  at com.kingsrook.qbits.backups.run.BackupEngine.doRun(BackupEngine.java:373)
  at BackupEngine.execute(171) <- BackupRunCoordinator.execute(334) <- runPlan(...) <- kof22 BackupPlanTicker

kof22 recently added qbit-edi 0.1.0-RC.9. Its edi_preview_download table sits on a storage-only backend (edi-preview-download, PreviewDownloadBackend). That backend implements QStorageInterface only, so getQueryInterface() throws. TableRoles.roleOf() only treated api backends as EXTERNAL, so this table was classified DATA, the keyset reader queried it, and the whole run died. Storage was not the problem: plan 2 writes to AWS and failed the same way. The run row only said Backup failed (IllegalStateException); see server logs for details. and error_detail was empty, so the cause could only be found in Loki.

What changed

Table selection

  • TableRoles.isQueryable(qInstance, table) looks up the table's backend module through QBackendModuleDispatcher and checks whether getQueryInterface() works. If it throws IllegalStateException or UnsupportedOperationException, the table is not queryable. The answer is cached per backend type because roleOf is called often. If no module can be resolved, the table is also treated as not queryable, but that answer is not cached because the module may register later.
  • roleOf returns EXTERNAL for such tables. This check is generic and does not depend on the table name. SECRET and BACKUPS still take precedence.
  • ScopeResolver always excludes these tables with the new reason NOT_QUERYABLE, including when they are selected by TABLE or ROLE rule or carry a declared role. Nothing can read them.
  • The Run backup review gets a line: "N tables are left out because their backend cannot be queried (storage only): <names>".
  • ExportTableProcess.exportProcesses skips them, so they get no per-table "Back up these records" action.

error_message / error_detail

  • New run/FailureText:
    • sanitize(String) redacts URL user-info, presigned query parameters (X-Amz-*, Signature, Credential, AWSAccessKeyId, *token*, key, ...), password=/secret=/token=/key=/accessKey=/secretKey= style values (plain and JSON), Authorization and Bearer/Basic values, AWS access key ids (AKIA/ASIA/...), PEM blocks, and Postgres Key (...)=(...) and Detail: / Failing row contains row values. Object keys and bucket names are kept. Every pattern is anchored so the cost stays linear on long messages.
    • summary(Throwable) produces Backup failed: <SimpleClass>: <sanitized message>, bounded to 500 characters.
    • detail(Throwable) gives the FQCN and sanitized message, then a Caused by: FQCN: sanitized message line for each cause (cycle-safe), then the top 8 frames of the root cause, bounded to 8 KB.
  • BackupEngine failure paths now record both values: the main FAILED path, the could-not-create-run-record path, and finish/finishWith, including their fallback. errorDetail goes to backups_run.errorDetail and to the new BackupRunResult.errorDetail. Success, cancel and timeout clear it, and so does a resume. The scrub() javadoc describes the new behaviour.

Tests

  • Reproduction (BackupEngineTest.testUnqueryableTableIsExcluded): registers a minimal storage-only backend module (StorageOnlyBackendModule, modelled on PreviewDownloadBackend) and a table on it in the test QInstance, then runs an INSTANCE-scope plan.
    • Before the fix, run against unfixed develop with the same setup: status=FAILED errorMessage=Backup failed (IllegalStateException); see server logs for details. errorDetail=null. This matches prod.
    • After the fix: the run ends SUCCEEDED_WITH_WARNINGS (the usual BEST_EFFORT warning), the manifest has no entry for the table, the role is EXTERNAL, and an explicit TABLE + ROLE=EXTERNAL scope still excludes it as NOT_QUERYABLE.
  • Failed put (testFailedPutRecordsSanitizedMessageAndDetail): storage throws BackupStorageException(ACCESS_DENIED). The exception carries a presigned MinIO URL with user-info, X-Amz-Credential=AKIA... and X-Amz-Signature, and its cause is IOException("XMinioStorageFull ... Authorization: ..."). The test asserts that the run row's errorMessage and errorDetail contain the class, the message, the object key, Caused by: java.io.IOException: XMinioStorageFull... and the root frames, and that the signature, the key id, the user-info and the auth values are redacted.
  • FailureTextTest: unit tests for every redaction rule, summary truncation, detail shape and bound, frames-less exceptions and cause cycles.
  • The two existing leak tests still assert that no credential or row value appears. They no longer assert the absence of https://, because hosts and object keys are now intentionally kept. The row value is now a Postgres Key (email)=(...) fragment.
  • ./mvnw -B verify: 217 tests, 0 failures, checkstyle and JaCoCo gates pass.

🤖 Generated with Claude Code

## Why kof22 health reports backups DOWN. Prod Loki logs show `backups_run` 5 and 6 (2026-10-16) both failed about 7 s in: ``` java.lang.IllegalStateException: Query is not implemented in this module: PreviewDownloadBackend at QBackendModuleInterface.throwNotImplemented(QBackendModuleInterface.java:254) at QBackendModuleInterface.getQueryInterface(QBackendModuleInterface.java:126) at QueryAction.executeInAuthorizedScope(QueryAction.java:259) at KeysetPager.fetchNextPage(KeysetPager.java:427) / forEachPage(259) at com.kingsrook.qbits.backups.run.TableReader$Keyset.read(TableReader.java:92) at com.kingsrook.qbits.backups.run.BackupEngine.doRun(BackupEngine.java:373) at BackupEngine.execute(171) <- BackupRunCoordinator.execute(334) <- runPlan(...) <- kof22 BackupPlanTicker ``` kof22 recently added qbit-edi 0.1.0-RC.9. Its `edi_preview_download` table sits on a storage-only backend (`edi-preview-download`, `PreviewDownloadBackend`). That backend implements `QStorageInterface` only, so `getQueryInterface()` throws. `TableRoles.roleOf()` only treated `api` backends as EXTERNAL, so this table was classified DATA, the keyset reader queried it, and the whole run died. Storage was not the problem: plan 2 writes to AWS and failed the same way. The run row only said `Backup failed (IllegalStateException); see server logs for details.` and `error_detail` was empty, so the cause could only be found in Loki. ## What changed **Table selection** - `TableRoles.isQueryable(qInstance, table)` looks up the table's backend module through `QBackendModuleDispatcher` and checks whether `getQueryInterface()` works. If it throws `IllegalStateException` or `UnsupportedOperationException`, the table is not queryable. The answer is cached per backend type because `roleOf` is called often. If no module can be resolved, the table is also treated as not queryable, but that answer is not cached because the module may register later. - `roleOf` returns **EXTERNAL** for such tables. This check is generic and does not depend on the table name. SECRET and BACKUPS still take precedence. - `ScopeResolver` always excludes these tables with the new reason `NOT_QUERYABLE`, including when they are selected by TABLE or ROLE rule or carry a declared role. Nothing can read them. - The Run backup review gets a line: "N tables are left out because their backend cannot be queried (storage only): `<names>`". - `ExportTableProcess.exportProcesses` skips them, so they get no per-table "Back up these records" action. **error_message / error_detail** - New `run/FailureText`: - `sanitize(String)` redacts URL user-info, presigned query parameters (`X-Amz-*`, `Signature`, `Credential`, `AWSAccessKeyId`, `*token*`, `key`, ...), `password=`/`secret=`/`token=`/`key=`/`accessKey=`/`secretKey=` style values (plain and JSON), `Authorization` and `Bearer`/`Basic` values, AWS access key ids (`AKIA`/`ASIA`/...), PEM blocks, and Postgres `Key (...)=(...)` and `Detail:` / `Failing row contains` row values. Object keys and bucket names are kept. Every pattern is anchored so the cost stays linear on long messages. - `summary(Throwable)` produces `Backup failed: <SimpleClass>: <sanitized message>`, bounded to 500 characters. - `detail(Throwable)` gives the FQCN and sanitized message, then a `Caused by: FQCN: sanitized message` line for each cause (cycle-safe), then the top 8 frames of the root cause, bounded to 8 KB. - `BackupEngine` failure paths now record both values: the main FAILED path, the could-not-create-run-record path, and `finish`/`finishWith`, including their fallback. `errorDetail` goes to `backups_run.errorDetail` and to the new `BackupRunResult.errorDetail`. Success, cancel and timeout clear it, and so does a resume. The `scrub()` javadoc describes the new behaviour. ## Tests - **Reproduction** (`BackupEngineTest.testUnqueryableTableIsExcluded`): registers a minimal storage-only backend module (`StorageOnlyBackendModule`, modelled on PreviewDownloadBackend) and a table on it in the test QInstance, then runs an INSTANCE-scope plan. - Before the fix, run against unfixed `develop` with the same setup: `status=FAILED errorMessage=Backup failed (IllegalStateException); see server logs for details. errorDetail=null`. This matches prod. - After the fix: the run ends `SUCCEEDED_WITH_WARNINGS` (the usual BEST_EFFORT warning), the manifest has no entry for the table, the role is EXTERNAL, and an explicit TABLE + ROLE=EXTERNAL scope still excludes it as `NOT_QUERYABLE`. - **Failed put** (`testFailedPutRecordsSanitizedMessageAndDetail`): storage throws `BackupStorageException(ACCESS_DENIED)`. The exception carries a presigned MinIO URL with user-info, `X-Amz-Credential=AKIA...` and `X-Amz-Signature`, and its cause is `IOException("XMinioStorageFull ... Authorization: ...")`. The test asserts that the run row's `errorMessage` and `errorDetail` contain the class, the message, the object key, `Caused by: java.io.IOException: XMinioStorageFull...` and the root frames, and that the signature, the key id, the user-info and the auth values are redacted. - `FailureTextTest`: unit tests for every redaction rule, summary truncation, detail shape and bound, frames-less exceptions and cause cycles. - The two existing leak tests still assert that no credential or row value appears. They no longer assert the absence of `https://`, because hosts and object keys are now intentionally kept. The row value is now a Postgres `Key (email)=(...)` fragment. - `./mvnw -B verify`: 217 tests, 0 failures, checkstyle and JaCoCo gates pass. 🤖 Generated with [Claude Code](https://claude.com/claude-code)
Sign in to join this conversation.