feat(java): signal retry intent by exception type#500
Conversation
A handler throwing RetryableException or NonRetryableException overrides the task retryOn predicate.
Every shell now derives its hidden-key list from the core instead of hardcoding one, so a missed prefix cannot reopen the hole in one binding.
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (4)
📝 WalkthroughWalkthroughThe PR centralizes reserved settings prefixes across Rust, Java, Node, and Python dashboard APIs, adds retention parse warnings, and introduces Java typed retry exceptions with centralized worker classification, documentation, and tests. ChangesCross-SDK reserved settings
Java typed retry signals
Estimated code review effort: 4 (Complex) | ~45 minutes Sequence Diagram(s)sequenceDiagram
participant TaskHandler
participant WorkerDispatchBridge
participant RetryDecision
participant JobBackend
TaskHandler->>WorkerDispatchBridge: throw typed or ordinary Throwable
WorkerDispatchBridge->>RetryDecision: isRetryable(retryOn, error)
RetryDecision->>RetryDecision: inspect cause chain or evaluate retryOn
RetryDecision-->>WorkerDispatchBridge: retryability result
WorkerDispatchBridge->>JobBackend: failJob(error, retryability)
Possibly related PRs
Suggested labels: Suggested reviewers: 🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (3 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
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. Comment |
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In
`@sdks/java/src/main/java/org/byteveda/taskito/dashboard/api/SettingsHandlers.java`:
- Around line 9-23: Remove eager NativeQueue.reservedSettingPrefixes()
evaluation from the static PROTECTED_PREFIXES initializer in SettingsHandlers.
Resolve the protected prefixes lazily or inject them so loading
SettingsHandlers, including InMemorySettings tests, does not trigger
NativeLoader; preserve consistent filtering once the prefixes are requested.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 9e0c9d6e-507f-433c-b1a0-c68121239ac6
📒 Files selected for processing (29)
crates/taskito-core/BINDING_CONTRACT.mdcrates/taskito-core/src/lib.rscrates/taskito-core/src/scheduler/retention.rscrates/taskito-core/src/settings.rscrates/taskito-java/src/queue/admin.rscrates/taskito-node/src/lib.rscrates/taskito-python/src/lib.rsdocs/content/docs/java/api-reference/errors.mdxdocs/content/docs/java/api-reference/task.mdxdocs/content/docs/shared/guides/reliability/error-handling.mdxdocs/content/docs/shared/guides/reliability/retries.mdxsdks/java/src/main/java/org/byteveda/taskito/dashboard/api/SettingsHandlers.javasdks/java/src/main/java/org/byteveda/taskito/errors/NonRetryableException.javasdks/java/src/main/java/org/byteveda/taskito/errors/RetryableException.javasdks/java/src/main/java/org/byteveda/taskito/errors/package-info.javasdks/java/src/main/java/org/byteveda/taskito/internal/NativeQueue.javasdks/java/src/main/java/org/byteveda/taskito/task/Task.javasdks/java/src/main/java/org/byteveda/taskito/worker/RetryDecision.javasdks/java/src/main/java/org/byteveda/taskito/worker/WorkerDispatchBridge.javasdks/java/src/test/java/org/byteveda/taskito/core/ExceptionHierarchyTest.javasdks/java/src/test/java/org/byteveda/taskito/dashboard/api/SettingsHandlersTest.javasdks/java/src/test/java/org/byteveda/taskito/worker/RetryDecisionTest.javasdks/java/src/test/java/org/byteveda/taskito/worker/TypedRetryExceptionTest.javasdks/node/src/dashboard/handlers/settings.tssdks/node/src/native.tssdks/node/test/dashboard/opsSettings.test.tssdks/python/taskito/_taskito.pyisdks/python/taskito/dashboard/handlers/settings.pysdks/python/tests/dashboard/test_dashboard_settings.py
Keeps dashboard class loading off the native library; the in-memory test store mirrors the core list, locked by a test.
Closes #484.
A Java handler could not say why it failed: everything thrown was classified the same way, and the only lever was the task-wide
retryOnpredicate added in #498.Typed retry signals (Java)
RetryableExceptionandNonRetryableExceptionjoinorg.byteveda.taskito.errors. A handler throws one to classify its own failure:retryOnpredicate — the throw site is more specific than a task-wide classifier.RetryDecision, so the bridge just asks it; unsignalled failures fall back to the predicate, and with no predicate everything still retries.Tests: unit coverage of the classification rules, plus end-to-end runs asserting a permanent failure dead-letters after one attempt and a retryable one exhausts its budget even under a rejecting predicate.
Follow-ups from #499
Two review comments on the retention-echo PR were never actioned:
taskito_core::RESERVED_SETTING_PREFIXESis the canonical list, exported by all three bindings; each shell's dashboard settings API derives its hidden-key list from it instead of hardcoding a slightly different one. Documented inBINDING_CONTRACT.md, with a per-shell test that the published retention document is neither listed nor writable.Verification
cargo test --workspace,cargo clippy --workspace --all-targetsclean:testsuite,:javadoc,spotlessCheckSummary by CodeRabbit
retryOn, including when wrapped in cause chains.retention:effective:default) across supported SDKs.