[smolkaj]

Currently, when a testcase's fuzzer no longer exists, preprocessing closes the testcase but then re-raises InvalidFuzzerError. The bot only drops tasks on InvalidTestcaseError, so the task message is never acked and gets redelivered until it expires, roughly a thousand failures per such testcase.

With this change, both errors derive from a new NonRetryableError (still an errors.Error, so metric labels are unchanged), and the bot drops tasks on any NonRetryableError, as suggested on b/578100711. This also covers corpus pruning tasks whose engine fuzzer is missing.

Dropped tasks now record their task status as ERROR, like any other failure; deleted-testcase tasks used to read FINISHED. For these tasks that status only acts as a lock against concurrent runs, so nothing else should change. Task.dont_retry() drops tasks along a second path; b/582875759 tracks settling on one.

This came out of an investigation of b/578100711 (done with AI help; report).

[IvanBM18]

LGTM, but, I would have this running a day in dev first and would use pitter to verify the before/after of this changes to see if everything is 'the same'.

To test this changes in the dev branch:

• Ask internally in the chat with GOSST if anyone is using dev, dev is a shared CF instance so we have to ask if no one else is running anything there before submitting changes to avoid from breaking someone else tests
• If dev is free, sync dev with master, commit your changes in dev & push them
• In around 20-30 min your changes should be reflected there
• You can verify which clusterfuzz version your bots are using by filtering the logs with jsonPayload.extras.clusterfuzz_version : '<first 5 chars of commit ID>`

[dylanjew]

this LGTM.

Are there other places we should loosen the catch to use NonRetryableError instead of the specific InvalidFuzzerError?

tagging GOSST on-duty @PauloVLB for approval.

[g-ortuno]

Non-blocking: Curious if you looked into the existing dont_retry mechanism. From a quick check it seems like we have two ways of doing the same thing and it might be worth aligning on a single one eventually. Might be worth opening a bug and leaving a TODO.

[PauloVLB]

Thanks for working on this!
LGTM. I'll wait for the dev validation @IvanBM18 proposed before approving.

[smolkaj]

@g-ortuno Good point, I hadn't looked at dont_retry(). I filed b/582875759 to settle on one mechanism and left a TODO pointing at it. I'd rather not fold it in here, since the bot loop's error path sleeps and exits the bot, which needs its own care.

@dylanjew I don't think so. The other places that catch InvalidFuzzerError (testcase setup, and fuzz and variant task preprocessing) handle a missing fuzzer specifically, e.g. by closing the testcase, so widening them would apply that handling to deleted testcases too. run_command is the one generic place that decides whether to retry.