fix(serverless): strict scheduling raises on dispatch failure, not just noop - #30
Conversation
…st noop create_task(..., strict=True) promised that scheduling failures surface to the caller, but strict only guarded one case: the LoggingNoop scheduler resolving in serverless mode. The AWS schedulers themselves never raised — AwsLambdaDeferredTaskScheduler swallowed invoke exceptions and returned a synthetic reference, returned early (warn only) when AWS_LAMBDA_FUNCTION_NAME was unset, and AwsSqsTaskScheduler silently no-op'd when unconfigured. A caller passing strict=True is stating that it has failure handling of its own — a 5xx to the webhook origin, a dedup claim to release — and that a silently-dropped task is data loss. Handing that caller a reference for a task that was never dispatched converts its retry path into exactly that loss: jvagent's WhatsApp deferred-interact webhook returned 200 on real dispatch failures, the wamid stayed claimed, and Meta's retry was dedup-blocked. `strict: bool = False` is added to TaskScheduler.schedule and threaded through dispatch_deferred_task. Under strict: aws_lambda raises on a missing function name and re-raises invoke failures (an EventBridge failure still falls back to direct invoke — a working fallback is not a dispatch failure); aws_sqs raises when unconfigured (send_message failures already propagated); the noop stub raises. Non-strict behaviour is byte-for-byte unchanged, so every existing fire-and-forget caller keeps its semantics. Tests cover both directions per path: strict raises where non-strict returns a synthetic ref, and the success path still returns a reference. 2014 passed, 129 skipped; pre-commit clean on touched files.
Benchmark comparisonThreshold: ±25% (informational, does not block merge)
|
|
Widening Blockers1.
Either forward 2. No CHANGELOG entry
Should-fix3. This is what The comment at 4. EventBridge failure silently downgrades a scheduled task to an immediate invoke —
5. The
6. Ad-hoc
7. Downstream-consumer specifics in library code and tests —
8. Contract docs not updated — The doc still describes the old narrow meaning: "raises 9. Coverage is Lambda-only No 10. Non-strict failure semantics differ by transport — Accurate as documented: SQS Nits
|
…transport Follow-up to the strict-scheduling change, closing the gaps found in review. Blocker: `dispatch_deferred_task` forwarded `strict=` to `sched.schedule()` unconditionally. `TaskScheduler` is a public/stable extension point and `config.task_scheduler` is duck-typed, so every third-party scheduler written against the pre-`strict` signature would have raised `TypeError` on *every* dispatch, non-strict included. The signature is now introspected (cached per class) and `strict` omitted for schedulers that predate it; a `strict=True` dispatch through one raises `TaskSchedulerNotConfiguredError` explaining it cannot honor the guarantee. Remaining silent-failure paths, all now raising under strict: * `NoopOrSyncScheduler` with no executor — what `get_task_scheduler` returns for every non-serverless caller — accepted `strict=True` and dropped the task. * Lambda `invoke` responses were never inspected. An async invoke answers 202 on acceptance, so a non-2xx `StatusCode` or a `FunctionError` was reported as success because boto3 had not raised. * An EventBridge scheduling failure fell back to invoking immediately with `process_at` in the body. Past Lambda's 900s timeout the handler cannot survive until `run_at`, so the task was doomed and the caller still got a success reference. * SQS `send_message` failures propagated while the Lambda transport swallowed them, so the same application code had opposite failure semantics depending on `JVSPATIAL_AWS_DEFERRED_TRANSPORT`. `strict` is now the single switch on both. Replace the ad-hoc `RuntimeError`s with `DeferredTaskError` → `TaskDispatchError` → `TaskSchedulerNotConfiguredError`, so a strict caller can distinguish "retry may succeed" from "this deployment will never dispatch". `DeferredTaskError` also derives from `RuntimeError`, keeping existing handlers working. Emit the one-time no-op diagnostic before the strict raise — a deployment whose callers are all strict never saw the startup error explaining why nothing dispatched. Add coverage for SQS, the logging no-op, the sync fallback, `strict` forwarding through the factory, legacy and `**kwargs` schedulers, and non-raising invoke rejections; the suite was Lambda-only. Rewrite the downstream-consumer narration in docstrings and test data generically per CLAUDE.md, and update `docs/md/serverless-mode.md` and SPEC §11.3, which still described the old narrow meaning of `strict`. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
|
Pushed Blockers
Should-fixes Nits — Full suite: 2038 passed, 129 skipped. Not addressed — pre-existing and outside this diff: downstream references in |
Benchmark comparisonThreshold: ±25% (informational, does not block merge)
|
1 participant
Summary
create_task(..., strict=True)promised scheduling failures surface to the caller, but strict guarded exactly one case: the LoggingNoop scheduler resolving in serverless mode. The AWS schedulers never raised —AwsLambdaDeferredTaskSchedulerswallowed invoke exceptions and returned a synthetic reference, returned early (warn only) withAWS_LAMBDA_FUNCTION_NAMEunset, andAwsSqsTaskSchedulersilently no-op'd when unconfigured.A strict caller is stating it has failure handling of its own — jvagent's WhatsApp webhook answers 5xx and releases its wamid dedup claim so Meta retries. A synthetic reference for an undispatched task turns that handling into silent message loss: the webhook returned 200, the wamid stayed claimed, and the retry was dedup-blocked.
Changes
strict: bool = Falseadded toTaskScheduler.scheduleand threaded throughdispatch_deferred_task.aws_lambdaraises on missing function name and re-raises invoke failures (EventBridge failure still falls back to direct invoke — a working fallback is not a dispatch failure);aws_sqsraises when unconfigured (send_messagealready propagated); the noop stub raises.Testing
2014 passed / 129 skipped; pre-commit clean on touched files. New tests cover both directions per path: strict raises where non-strict returns a synthetic ref; the success path still returns a reference.
Consumer side: TrueSelph/jvagent#132 carries a contract test that pins these semantics against the installed jvspatial and skips with a loud deploy warning when paired with a version predating this fix.
🤖 Generated with Claude Code