Skip to content

fix(plugins): handle initial LOADING persistence failures - #104

Merged
fronzec merged 1 commit into
mainfrom
fix/plugin-loading-initial-save
Oct 17, 2026
Merged

fronzec merged 1 commit into
mainfrom
fix/plugin-loading-initial-save

Conversation

@fronzec

@fronzec fronzec commented Oct 17, 2026 •

Copy link
Copy Markdown
Owner

Persist the LOADING transition inside the load failure boundary so a
failed save ends in FAILED with the original exception as cause,
records failure-persistence errors as suppressed, emits a FAILURE
audit event, and creates no classloader or registry ownership.

Closes #95

Summary by CodeRabbit

  • Bug Fixes
    • Job loading failures during the initial status update are now handled consistently: the job is marked as failed, the error is recorded, and failure auditing is performed.
    • A failed attempt to save the failure status no longer obscures the original load error, and jobs can be retried after an initial status-save failure.

Persist the LOADING transition inside the load failure boundary so a
failed save ends in FAILED with the original exception as cause,
records failure-persistence errors as suppressed, emits a FAILURE
audit event, and creates no classloader or registry ownership.

Closes #95

fronzec commented Oct 17, 2026

Copy link
Copy Markdown
Owner Author

This stack of pull requests is managed by Graphite. Learn more about stacking.

@coderabbitai

coderabbitai Bot commented Oct 17, 2026 •

Copy link
Copy Markdown
Contributor

Review in Change Stack →

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration
  • Configuration used: defaults
  • Review profile: CHILL
  • Plan: Advanced
  • Run ID: 6882e5bb-ea8d-4e71-803c-44ce83bedf48
📥 Commits

Reviewing files that changed from the base of the PR and between 27382eb and a76707f.

📒 Files selected for processing: 2
  • fr-batch-service/src/main/java/com/fronzec/frbatchservice/batchjobs/plugins/loader/DynamicJobLoaderService.java
  • fr-batch-service/src/test/java/com/fronzec/frbatchservice/batchjobs/plugins/loader/DynamicJobLoaderServiceTest.java

Included review availability: This review used your included allowance. Your plan provides up to 1 included review per hour; 0 remain after this review.

📝 Walkthrough

The initial LOADING state update now runs inside the loader’s failure-handling block. Tests cover failure recording, cleanup, a failed attempt to persist FAILED, and a successful retry.

Changes: Load failure handling

Layer / File(s) Summary
Handle initial state persistence failures
fr-batch-service/src/main/java/com/fronzec/frbatchservice/batchjobs/plugins/loader/DynamicJobLoaderService.java, fr-batch-service/src/test/java/com/fronzec/frbatchservice/batchjobs/plugins/loader/DynamicJobLoaderServiceTest.java
The loader handles a failed initial LOADING save through its existing failure path. Tests check the original cause, failure status and error, audit event, absence of classloader and plugin registration, lifecycle-state cleanup, suppressed failure-persistence errors, and successful retry.

Priority: ➖ Normal

Estimated code review effort: 2 (Simple) | ~10 minutes

Change: Bug fix · Severity of issue fixed: Medium

Merge Risk: ⚪ Minimal · up to a7670

The change routes a failed initial LOADING save through the existing failure handling, and tests cover cleanup, suppressed errors and retry. No concrete merge-blocking risk remains.

Pre-merge checks: Passed 4 · Failed 1

Failed Failed checks: 1 warning

Check name Status Explanation Resolution
Docstring Coverage Warning Docstring coverage is 50.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 6 functions across 2 files. Write docstrings for the functions missing them to satisfy the coverage threshold.
Passed Passed checks: 4 passed
Check name Status Explanation
Description Check Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check Passed The title clearly and concisely describes the main change: handling failures when persisting the initial LOADING state.
Linked Issues check Passed The change in DynamicJobLoaderService.doLoadJob moves the initial LOADING mutation and repository save inside the existing failure boundary. The failure path sets FAILED, stores the original exc…
Out of Scope Changes check Passed The pull request changes only the loader failure boundary and focused tests for initial LOADING persistence failures. The tests support issue #95 requirements. No unrelated lifecycle, unload, DELETE…
Full details: Linked Issues check

Explanation

The change in DynamicJobLoaderService.doLoadJob moves the initial LOADING mutation and repository save inside the existing failure boundary. The failure path sets FAILED, stores the original exception message in loadError, preserves the original exception as the JobLoadException cause, and adds a failure-persistence exception as suppressed. It emits the existing AuditEvent.FAILURE event. The focused tests cover initial-save failure, failed FAILED persistence, lock release, no classloader or registry ownership, and retry recovery for issue #95.

Full details: Out of Scope Changes check

Explanation

The pull request changes only the loader failure boundary and focused tests for initial LOADING persistence failures. The tests support issue #95 requirements. No unrelated lifecycle, unload, DELETE, distributed-locking, or registry behavior changes are shown.

  • Fix all pre-merge checks with AI
✨ Finishing Touches: 💡 1
📝 Generate docstrings: 💡
  • Commit to this branch
  • Create a new PR
🧪 Generate unit tests (beta)
  • Commit to this branch
  • Create a new PR
  • Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@fronzec
fronzec merged commit 746f885 into main Oct 17, 2026
8 checks passed
@fronzec
fronzec deleted the fix/plugin-loading-initial-save branch October 17, 2026 01:14
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.

fix(plugin): handle initial LOADING persistence failures

1 participant