Conversation
Every element of a job's `errors` has to decode into `rivertype.AttemptError` for the job row to be readable. An element that's valid JSON but not in the shape River writes, like an `at` in Postgres' text timestamp format, an `attempt` stored as a string, or a structured `error` or `trace` written by another tool, fails the whole row. When the row is one of the jobs locked by a fetch, the fetch returns an error after the claim has already moved every locked job to `running`, so all of them sit there without being worked, and the rescuer fails on the same row when it tries to recover them. Give `AttemptError` an `UnmarshalJSON` that keeps encoding/json's behavior for well-formed elements and falls back to best effort decoding for any other valid JSON: `at` also accepts a few common timestamp variants and is otherwise left zero, `attempt` accepts integral numbers and numeric strings, non-string `error` and `trace` values are kept as their JSON text, and a non-object element becomes the error message. Only invalid JSON still returns an error, which a Postgres `jsonb[]` element can never be. The persisted format doesn't change. Unit tests cover each fallback, and a new driver test fetches a job with oddly shaped attempt errors alongside a normal one on every driver.
The fetch query moves every job it locks to `running` before the driver decodes the returned rows, and one row that can't be decoded fails the whole fetch. The producer only logs that error, so every job locked alongside the bad one is left `running` without being worked. The rescuer can't recover them either, since it reads stuck jobs with the same strict decoding and fails on the same row. After lenient attempt error decoding this can no longer happen on Postgres, but on SQLite `errors`, `tags`, and `attempted_by` are JSON columns that can be changed to any shape. Have `JobGetAvailable` decode each locked row separately and return a `JobGetAvailableResult` with the jobs that decoded and, separately, the `UndecodableJobs` along with the fields that could be decoded and the decode error. The pilot passes the result through, and the producer starts an executor for each undecodable job with the decode error set. The executor doesn't work such a job. It fails the attempt with an error describing the decode failure before hooks or middleware run, the same way it handles an unknown job kind, so the error handler is invoked and the job is retried with the client's retry policy or discarded at its max attempts, emitting a failed event. The attempt error is appended in SQL without decoding the bad column, and the bad values are left as they were. On SQLite, an `errors` value that isn't an array is wrapped in one so the new error can still be appended. `JobSetStateIfRunningMany` and `JobGetStuck` return rows that can't be fully decoded with the undecodable fields left empty instead of failing, so completing an undecodable job doesn't fail (or on SQLite, roll back) the rest of its batch, and the rescuer can recover jobs stranded by this problem before upgrading. Driver tests cover each of these on SQLite, and client tests show good jobs completing alongside undecodable ones that end up `retryable` or `discarded` with the decode error. Producer and executor tests exercise the same path on Postgres using a pilot that reports a job as undecodable.
bgentry
force-pushed
the
bg/isolate-undecodable-jobs
branch
from
September 24, 2026 18:00
f3a4d07 to
812094b
Compare
A job whose attempt fails because its row can't be decoded is moved to `retryable` with its values left in place. On SQLite, the scheduler decodes every job it schedules strictly, so once such a job's retry comes due, each scheduler run fails on it and rolls back the whole batch. No retryable or scheduled job gets scheduled again until the row is repaired, and a job discarded for a unique conflict fails the same way. Decode the rows the scheduler updates the same way as stuck jobs, with the fields that can't be decoded left empty instead of failing, so an undecodable job is scheduled or discarded along with the others and comes back through the fetch path to be retried or discarded. A driver test schedules undecodable retryable and scheduled jobs, and one that's discarded for a unique conflict, alongside a normal job.
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.
When River fetches jobs, it first moves every job it locks to
runningand only then decodes the rows. If a single row won't decode, the whole fetch fails. That can happen with an attempt error stored as{"attempt": "1", ...}instead of a number, or a SQLite job whosetagswere changed to an object. The producer just logs the error, and the rescuer trips over the same row when it tries to recover the stuck jobs, so every job that was locked alongside the bad one staysrunningindefinitely and never gets worked.Attempt errors now decode leniently: any valid JSON element is read on a best-effort basis, and the stored format doesn't change. Rows that still can't be decoded are returned separately by the driver. Their attempt fails with an error describing the decode failure, and they're retried or discarded like any other failed job, while the jobs fetched with them run normally. The bad values are left in place so nothing is lost. Completion, the rescuer, and the SQLite scheduler also tolerate such rows, so one bad row can't take down a batch. Without the scheduler change, once an undecodable job on SQLite came due for its retry, every scheduler run would fail on it and roll back, and no retryable or scheduled job in the database would get scheduled again.
riverpilot.Pilot.JobGetAvailablenow returns a*riverdriver.JobGetAvailableResultcarrying both the decoded jobs and the undecodable ones, so custom pilot implementations need a small update.