Skip to content

Improve scope error handling#873

Merged
ahal merged 2 commits into
taskcluster:mainfrom
ahal:ahal/push-rkmlwsluyvzr
Dec 19, 2025
Merged

Improve scope error handling#873
ahal merged 2 commits into
taskcluster:mainfrom
ahal:ahal/push-rkmlwsluyvzr

Conversation

@ahal

@ahal ahal commented Dec 8, 2025

Copy link
Copy Markdown
Contributor

No description provided.

@ahal ahal self-assigned this Dec 8, 2025
@ahal
ahal force-pushed the ahal/push-rkmlwsluyvzr branch 13 times, most recently from 2946aee to a297b24 Compare December 12, 2025 17:38
@ahal
ahal marked this pull request as ready for review December 12, 2025 17:38
@ahal
ahal requested a review from a team as a code owner December 12, 2025 17:38
@ahal
ahal requested a review from Eijebong December 12, 2025 17:38
@ahal
ahal marked this pull request as draft December 12, 2025 18:43
@ahal
ahal force-pushed the ahal/push-rkmlwsluyvzr branch 2 times, most recently from 9edbb74 to b1605da Compare December 12, 2025 21:40
@ahal
ahal marked this pull request as ready for review December 12, 2025 21:46
ahal added 2 commits December 15, 2025 09:19
This allows us to get more coverage out of these tests.
This prints the label of the task we failed to create for convenience,
as well as defers failing the Decision task until after we've attempted
all tasks. This way we'll see all the scope errors at once rather than
needing to fix them one at a time.
@ahal
ahal force-pushed the ahal/push-rkmlwsluyvzr branch from b1605da to eb3fee6 Compare December 15, 2025 14:19

@abhishekmadan30 abhishekmadan30 left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@ahal
ahal merged commit 76511bd into taskcluster:main Dec 19, 2025
19 checks passed
@ahal
ahal deleted the ahal/push-rkmlwsluyvzr branch December 19, 2025 14:10
bhearsum added a commit to bhearsum/taskgraph that referenced this pull request Jun 30, 2026
I noticed this as an intermittent test failure in CI, which looks like a minor regression from taskcluster#873:
```
[task 2026-06-29T11:51:43.891+00:00] ______________ TestCreate.test_create_tasks_fails_if_create_fails ______________
[task 2026-06-29T11:51:43.891+00:00] 
[task 2026-06-29T11:51:43.891+00:00] self = <test.test_create.TestCreate testMethod=test_create_tasks_fails_if_create_fails>
[task 2026-06-29T11:51:43.891+00:00] 
[task 2026-06-29T11:51:43.891+00:00]     @responses.activate
[task 2026-06-29T11:51:43.891+00:00]     @mock.patch.dict(
[task 2026-06-29T11:51:43.891+00:00]         "os.environ",
[task 2026-06-29T11:51:43.891+00:00]         {"TASKCLUSTER_ROOT_URL": "https://tc.example.com"},
[task 2026-06-29T11:51:43.891+00:00]         clear=True,
[task 2026-06-29T11:51:43.891+00:00]     )
[task 2026-06-29T11:51:43.891+00:00]     def test_create_tasks_fails_if_create_fails(self):
[task 2026-06-29T11:51:43.891+00:00]         "create_tasks fails if a single create_task call fails"
[task 2026-06-29T11:51:43.891+00:00]         mock_taskcluster_api(error_status=403, error_message="oh no!")
[task 2026-06-29T11:51:43.891+00:00]     
[task 2026-06-29T11:51:43.891+00:00]         tasks = {
[task 2026-06-29T11:51:43.891+00:00]             "tid-a": Task(
[task 2026-06-29T11:51:43.891+00:00]                 kind="test", label="a", attributes={}, task={"payload": "hello world"}
[task 2026-06-29T11:51:43.891+00:00]             ),
[task 2026-06-29T11:51:43.891+00:00]         }
[task 2026-06-29T11:51:43.891+00:00]         label_to_taskid = {"a": "tid-a"}
[task 2026-06-29T11:51:43.891+00:00]         graph = Graph(nodes={"tid-a"}, edges=set())
[task 2026-06-29T11:51:43.891+00:00]         taskgraph = TaskGraph(tasks, graph)
[task 2026-06-29T11:51:43.891+00:00]     
[task 2026-06-29T11:51:43.891+00:00] >       with self.assertRaises(CreateTasksException):
[task 2026-06-29T11:51:43.891+00:00]              ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
[task 2026-06-29T11:51:43.891+00:00] E       AssertionError: CreateTasksException not raised
```

It's probably impossible to hit in any non-trivial taskgraph in production, but it's certainly something that *could* happen, particularly in smaller/shallower graphs.

The fix is moving the error checking out to the main flow of control, and doing it synchronously. This leaves `handle_exception` doing nothing except populated `skipped`, so I moved it into `schedule_tasks` and renamed it to make that a bit more obvious.
bhearsum added a commit that referenced this pull request Jun 30, 2026
I noticed this as an intermittent test failure in CI, which looks like a minor regression from #873:
```
[task 2026-06-29T11:51:43.891+00:00] ______________ TestCreate.test_create_tasks_fails_if_create_fails ______________
[task 2026-06-29T11:51:43.891+00:00] 
[task 2026-06-29T11:51:43.891+00:00] self = <test.test_create.TestCreate testMethod=test_create_tasks_fails_if_create_fails>
[task 2026-06-29T11:51:43.891+00:00] 
[task 2026-06-29T11:51:43.891+00:00]     @responses.activate
[task 2026-06-29T11:51:43.891+00:00]     @mock.patch.dict(
[task 2026-06-29T11:51:43.891+00:00]         "os.environ",
[task 2026-06-29T11:51:43.891+00:00]         {"TASKCLUSTER_ROOT_URL": "https://tc.example.com"},
[task 2026-06-29T11:51:43.891+00:00]         clear=True,
[task 2026-06-29T11:51:43.891+00:00]     )
[task 2026-06-29T11:51:43.891+00:00]     def test_create_tasks_fails_if_create_fails(self):
[task 2026-06-29T11:51:43.891+00:00]         "create_tasks fails if a single create_task call fails"
[task 2026-06-29T11:51:43.891+00:00]         mock_taskcluster_api(error_status=403, error_message="oh no!")
[task 2026-06-29T11:51:43.891+00:00]     
[task 2026-06-29T11:51:43.891+00:00]         tasks = {
[task 2026-06-29T11:51:43.891+00:00]             "tid-a": Task(
[task 2026-06-29T11:51:43.891+00:00]                 kind="test", label="a", attributes={}, task={"payload": "hello world"}
[task 2026-06-29T11:51:43.891+00:00]             ),
[task 2026-06-29T11:51:43.891+00:00]         }
[task 2026-06-29T11:51:43.891+00:00]         label_to_taskid = {"a": "tid-a"}
[task 2026-06-29T11:51:43.891+00:00]         graph = Graph(nodes={"tid-a"}, edges=set())
[task 2026-06-29T11:51:43.891+00:00]         taskgraph = TaskGraph(tasks, graph)
[task 2026-06-29T11:51:43.891+00:00]     
[task 2026-06-29T11:51:43.891+00:00] >       with self.assertRaises(CreateTasksException):
[task 2026-06-29T11:51:43.891+00:00]              ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
[task 2026-06-29T11:51:43.891+00:00] E       AssertionError: CreateTasksException not raised
```

It's probably impossible to hit in any non-trivial taskgraph in production, but it's certainly something that *could* happen, particularly in smaller/shallower graphs.

The fix is moving the error checking out to the main flow of control, and doing it synchronously. This leaves `handle_exception` doing nothing except populated `skipped`, so I moved it into `schedule_tasks` and renamed it to make that a bit more obvious.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants