feat(core): whole-job deadline + scrub driver-exception messages from run error (rec #5) - #230
Merged
Merged
Conversation
… the run error Two enterprise-readiness items from the audit. Whole-job deadline (complements WP6's per-attempt read timeout): ReportBuilder<T>. Deadline(TimeSpan) sets an overall wall-clock bound on the entire run — reads, writes and uploads together — so a report that never hangs on a single step but drags on overall is still stopped. RunAsync applies it via a linked CancellationTokenSource + CancelAfter; on expiry the run is cooperatively cancelled (surfaces as a cancelled run) and a warning distinguishes a deadline expiry from a caller-requested cancel. Off by default. Applied on both production entry paths (the job worker and the sync endpoint both call RunAsync). Error scrub at the source: the run's persisted error (job.Error, surfaced by GET /jobs), the RunFailed and Retry job events, and the sync-run error previously carried the raw exception message — for a driver exception that can echo the connection string (host/port/database/username). Now NeoReports' own exceptions keep their curated, secret-free message (e.g. "No source named 'x'") and any other exception is reduced to its type name; the full exception is still logged through ILogger for diagnosis. Applied consistently at every site that persists a failure reason: AbortStrategy, the runner's read-failure/retry paths, and the job worker's catch (which handles exceptions that escape the runner during setup). This closes the async-path leak the WP5 endpoint scrub had left, without genericizing the whole job timeline. Tests: deadline cancels a slow run promptly and a no-deadline run completes; the driver message is gone from the run error (abort and skip-read-failure paths) while a NeoReports message survives.
|
|
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.
Fifth maintainer-decision recommendation — two enterprise-readiness items.
A) Whole-job deadline
Complements WP6's per-attempt read timeout.
ReportBuilder<T>.Deadline(TimeSpan)sets an overall wall-clock bound on the entire run — reads, writes and uploads together — so a report that never hangs on any single step but drags on overall is still stopped.RunAsyncapplies it via a linkedCancellationTokenSource+CancelAfter; on expiry the run is cooperatively cancelled (surfaces as a cancelled run) and a warning distinguishes a deadline expiry from a caller-requested cancel. Off by default. Applied on both production entry paths (the job worker and the sync endpoint both callRunAsync; nothing in production callsExecuteAsyncdirectly).B) Error scrub at the source (the WP5 async-path follow-up)
The run's persisted error (
job.Error, surfaced byGET /jobs), theRunFailedandRetryjob events, and the sync-run error previously carried the raw exception message — for a driver exception, that can echo the connection string (host/port/database/username). Now:No source named 'sales-db').SqlException,SocketException, …).ILoggerfor diagnosis (WP7).Applied consistently at every site that persists a failure reason:
AbortStrategy, the runner's read-failure and retry paths, and the job worker's catch (which handles exceptions that escape the runner during setup — the completeness gap review found). This closes the async-path leak WP5's endpoint scrub had left, without genericizing the whole job timeline (the maintainer-judgment concern from WP5): the timeline still shows a useful reason, just secret-free.Tests
Deadline cancels a slow run promptly (100ms deadline stops a 5s-per-page source) and a no-deadline run completes; the driver message is gone from the run error on both the abort and skip-read-failure paths, while a NeoReports message survives.
Verification
dotnet build --no-incremental— 0 warnings, 0 errors.Recs #1–#4 merged (#226–#229). This is #5. Last up: the minor items (#6) — CA1068 CancellationToken ordering, CI fail-not-skip for Docker-absent integration tests, XLSX number-format assertions (already added in #226), and the drain-helper page caps.