fix(cron): treat non-dict origin as missing instead of crashing tick#19013
Closed
Tranquil-Flow wants to merge 1 commit into
Closed
fix(cron): treat non-dict origin as missing instead of crashing tick#19013Tranquil-Flow wants to merge 1 commit into
Tranquil-Flow wants to merge 1 commit into
Conversation
``_resolve_origin`` called ``origin.get('platform')`` on whatever
``job.get('origin')`` returned. The leading ``if not origin: return None``
short-circuited the falsy cases (None, empty dict, "") but a non-empty
string passed that guard and then crashed with
``AttributeError: 'str' object has no attribute 'get'`` on every fire
attempt. Observed in the wild after a migration script tagged jobs with
free-form provenance strings (e.g.
``"combined-digest-replaces-x-and-y-20260503"``).
``mark_job_run`` did record ``last_status: error,
last_error: "'str' object has no attribute 'get'"`` once, but the next
tick re-loaded the same poisoned origin and crashed identically. The
job stayed enabled, fired every tick, and accumulated cascading errors
in the log until ``origin`` was patched manually.
Replace the falsy guard with ``isinstance(origin, dict)``. Non-dict
origins (string, int, list, tuple, float — anything that survived a
hand-edit, JSON-script write, or migration) are now treated the same
as a missing origin: the job continues with ``deliver`` falling back
through its normal home-channel path instead of crashing the scheduler
loop.
Test parametrises the non-dict shapes that can appear in jobs.json
through external writers and asserts ``_resolve_origin`` returns None
for each.
Note: this fix scope is the non-dict-``origin`` crash only. The
``next_run_at: null`` recurring-job recovery (the second sub-bug in
NousResearch#18722) is independently addressed by the in-flight NousResearch#18825, which
extends the never-silently-disable defense from NousResearch#16265 to
``get_due_jobs()`` — that approach is well-aligned with the existing
recovery pattern and ships fine without a competing change here.
Fixes NousResearch#18722 (non-dict origin crash; recurring-job recovery covered by NousResearch#18825)
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
2 participants
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.
What does this PR do?
_resolve_origincalledorigin.get('platform')on whateverjob.get('origin')returned. The leadingif not origin: return Noneshort-circuited the falsy cases (None, empty dict, "") but a non-empty string passed that guard and crashed withAttributeError: 'str' object has no attribute 'get'on every fire attempt. Observed in the wild after a migration script tagged jobs with free-form provenance strings (e.g."combined-digest-replaces-x-and-y-20260503").mark_job_rundid recordlast_status: error, last_error: "'str' object has no attribute 'get'"once, but the next tick re-loaded the same poisoned origin and crashed identically. The job stayed enabled and accumulated cascading errors untiloriginwas patched manually.Replace the falsy guard with
isinstance(origin, dict). Non-dict origins (string, int, list, tuple, float — anything that survived a hand-edit, JSON-script write, or migration) are now treated the same as a missing origin: the job continues withdeliverfalling back through its normal home-channel path instead of crashing the scheduler loop.Scope: the non-dict-
origincrash sub-bug from #18722. Thenext_run_at: nullrecurring-job recovery (the second sub-bug) is independently addressed by the in-flight #18825, which extends the never-silently-disable defense from #16265 toget_due_jobs(). Either one can land first.Related Issue
Fixes #18722 (non-dict origin crash; recurring-job recovery covered by #18825)
Type of Change
Changes Made
cron/scheduler.py—_resolve_originguardsisinstance(origin, dict)before.get()calls; updated docstring with the production trigger pattern.tests/cron/test_scheduler.py—TestResolveOrigin.test_non_dict_origin_returns_none_instead_of_crashingparametrises over the non-dict shapes (str, int, list, tuple, float).How to Test
originto a string like"my-migration-tag".AttributeError; after: job runs with default delivery routing.Checklist
Code
pytest tests/ -qand the touched suite passesDocumentation & Housekeeping
cli-config.yaml.exampleif I added/changed config keys — N/ACONTRIBUTING.mdorAGENTS.mdif I changed architecture — N/A