Uh oh!
There was an error while loading. Please reload this page.
Fix Edge worker reporting crashed tasks as SUCCESS - #65833
Closed
dheerajturaga wants to merge 1 commit into
Closed
Conversation
_run_job_via_supervisor returned 1 on caught exception, but multiprocessing.Process ignores the target's return value — exit code was always 0, so Job.is_success was True and crashed tasks were reported to the central server as SUCCESS despite the Task execution failed traceback in the log. Use sys.exit(1) so the subprocess exits with code 1 and fetch_and_run_job takes the FAILED branch.
eladkal
approved these changes
Apr 25, 2026
jscheffl
approved these changes
Apr 25, 2026
Contributor
There was a problem hiding this comment.
Okay, signalling is better but I do not see an effect. At least folliowing my "integration test" Dag also a failed task ("plan_to_fail") still is reported in "success":
So as I also see this as an improvement, can you tell where you see an effect in reporting the exit status?
MemberAuthor
@jscheffl, What you are reporting seems to be a different bug altogether. when a user task raises, task_runner.main() catches it, sends TaskState(FAILED) to the supervisor, and returns normally. _fork_main then exits the task-runner subprocess with code 0. supervise_task returns exit_code=0. edge3's _run_job_via_supervisor only looks at the return value, so Job.is_success is True and it calls jobs_set_state(..., SUCCESS) . I will raise another followup PR for the above. what I was trying to address in this PR is the following exception |
jscheffl
commented
Apr 26, 2026
Contributor
So in your case the task went down in a HTTP exception and then the worker expected "all is good". But in such case of failure if the API server is down (even if retries), then it can not report back to central instance anyway? Or do you have other /longer retries configured for the worker compared to the execution API? Because if consistently configured then either both workers as well as task retry until API server is back or anyway both will fail communicating back to origin. Handling the error "just" changes the logs cosmetically then? |
jscheffl
commented
Jun 2, 2026
Contributor
@dheerajturaga just merged PR #67887 - did this maybe resolve the reason for this PR already? |
dheerajturaga
commented
Jun 2, 2026
MemberAuthor
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 freeto join this conversation on GitHub.
Already have an account?
Sign in to comment
3 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.
_run_job_via_supervisor returned 1 on caught exception, but
multiprocessing.Process ignores the target's return value — exit code
was always 0, so Job.is_success was True and crashed tasks were
reported to the central server as SUCCESS despite the Task execution
failed traceback in the log. Use sys.exit(1) so the subprocess exits
with code 1 and fetch_and_run_job takes the FAILED branch.
Was generative AI tooling used to co-author this PR?
ClaudeCode Opus 4.7