Skip to content

worker: a default-configured Worker claims job types it has no handler for, and destroys them #246

Description

@jerelvelarde

Found during a review round on #224. Latent in production today, one config edit away from live.

What happens

claimAndProcessJobs claims PENDING rows with no type predicate — unlike claimJobsForType, which has AND type = ${type}. processJob then finds no handler and writes a terminal status: 'FAILED' tombstone.

FAILED is not PROCESSING, so reclaimStaleJobs will never touch it. Nothing else in the queue package re-queues FAILED rows, and handlers/job-cleanup.ts deletes only COMPLETED and DEAD_LETTER, so the row is not even reaped. The job is simply gone, with one console.warn.

Why it is not live today

apps/worker/src/index.ts sets concurrencyByType for all ten types, so hasPerTypeLimits is true and only claimJobsByType runs — and that iterates this.handlers, so it never claims a type it cannot handle.

But concurrencyByType is optional in WorkerOptions. A Worker constructed without it — the constructor default, the shape used throughout the tests, and what any other consumer of the exported Worker gets — takes the destroying path.

Why it matters beyond the default

Heterogeneous replicas are an explicitly anticipated state. reclaimStaleJobs's own docstring reasons about "a crashed claim of a type this replica does not handle", and #224 made the sweep deliberately type-blind for exactly that reason. The claim side has the opposite requirement and says nothing about it. A rolling deploy where a new revision enqueues a type the old revision lacks loses those jobs on the default config.

Suggested fix

Either constrain the batch claim to registered types:

AND type = ANY(${Array.from(this.handlers.keys())})

or make the no-handler path release the row back to PENDING with no attempt consumed, rather than tombstoning it.

Note the second option is a deliberate behaviour change, not an oversight fix: handles missing handler gracefully in queue.test.ts currently asserts the tombstone, so that test has to change with it.

Related

  • FAILED rows are never reaped by JOB_CLEANUP — worth closing at the same time, since the tombstone is the only writer of FAILED.

Activity

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    roadmap: nowRoadmap horizon: V1 launch critical path / safety

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions