From 34816458276bf9a528f306f0bc26f33dfdb428c8 Mon Sep 17 00:00:00 2001 From: Stephanie Kim Date: Mon, 14 Mar 2022 21:04:02 +0000 Subject: [PATCH] Check if json exists in ensure_checkout's finally block The step_result json can currently be empty if the orchestrator build is canceled while running bot_update. Trying to do step_result.json.output in the finally block raises an "Uncaught exception AttributeError". Ex: https://ci.chromium.org/ui/p/chromium/builders/try/win10_chromium_x64_rel_ng/1121568/overview This change checks for the json attr before trying to read json.output. Bug: 1305332 Change-Id: I1dc277642ece1b96144be6381a7c4b8426cb2bc6 Reviewed-on: https://chromium-review.googlesource.com/c/chromium/tools/depot_tools/+/3518372 Reviewed-by: Garrett Beaty Reviewed-by: Josip Sokcevic Commit-Queue: Stephanie Kim --- recipes/README.recipes.md | 10 +++++----- recipes/recipe_modules/bot_update/api.py | 8 +++++++- recipes/recipe_modules/bot_update/examples/full.py | 7 +++++++ 3 files changed, 19 insertions(+), 6 deletions(-) diff --git a/recipes/README.recipes.md b/recipes/README.recipes.md index 216ff28352..794cab800c 100644 --- a/recipes/README.recipes.md +++ b/recipes/README.recipes.md @@ -59,7 +59,7 @@ Recipe module to ensure a checkout is consistent on a bot. Wrapper for easy calling of bot_update. -— **def [deapply\_patch](/recipes/recipe_modules/bot_update/api.py#516)(self, bot_update_step):** +— **def [deapply\_patch](/recipes/recipe_modules/bot_update/api.py#522)(self, bot_update_step):** Deapplies a patch, taking care of DEPS and solution revisions properly. @@ -93,7 +93,7 @@ Args: bot_update module ONLY supports one change. Users may specify a change via tryserver.set_change() and explicitly set this flag False. -— **def [get\_project\_revision\_properties](/recipes/recipe_modules/bot_update/api.py#493)(self, project_name, gclient_config=None):** +— **def [get\_project\_revision\_properties](/recipes/recipe_modules/bot_update/api.py#499)(self, project_name, gclient_config=None):** Returns all property names used for storing the checked-out revision of a given project. @@ -109,7 +109,7 @@ Returns (list of str): All properties that'll hold the checked-out revision   **@property**
— **def [last\_returned\_properties](/recipes/recipe_modules/bot_update/api.py#49)(self):** -— **def [resolve\_fixed\_revision](/recipes/recipe_modules/bot_update/api.py#444)(self, bot_update_json, name):** +— **def [resolve\_fixed\_revision](/recipes/recipe_modules/bot_update/api.py#450)(self, bot_update_json, name):** Sets a fixed revision for a single dependency using project revision properties. @@ -988,11 +988,11 @@ Raises: ### *recipes* / [bot\_update:examples/full](/recipes/recipe_modules/bot_update/examples/full.py) -[DEPS](/recipes/recipe_modules/bot_update/examples/full.py#7): [bot\_update](#recipe_modules-bot_update), [gclient](#recipe_modules-gclient), [gerrit](#recipe_modules-gerrit), [tryserver](#recipe_modules-tryserver), [recipe\_engine/buildbucket][recipe_engine/recipe_modules/buildbucket], [recipe\_engine/json][recipe_engine/recipe_modules/json], [recipe\_engine/path][recipe_engine/recipe_modules/path], [recipe\_engine/platform][recipe_engine/recipe_modules/platform], [recipe\_engine/properties][recipe_engine/recipe_modules/properties], [recipe\_engine/runtime][recipe_engine/recipe_modules/runtime] +[DEPS](/recipes/recipe_modules/bot_update/examples/full.py#9): [bot\_update](#recipe_modules-bot_update), [gclient](#recipe_modules-gclient), [gerrit](#recipe_modules-gerrit), [tryserver](#recipe_modules-tryserver), [recipe\_engine/buildbucket][recipe_engine/recipe_modules/buildbucket], [recipe\_engine/json][recipe_engine/recipe_modules/json], [recipe\_engine/path][recipe_engine/recipe_modules/path], [recipe\_engine/platform][recipe_engine/recipe_modules/platform], [recipe\_engine/properties][recipe_engine/recipe_modules/properties], [recipe\_engine/runtime][recipe_engine/recipe_modules/runtime] PYTHON_VERSION_COMPATIBILITY: PY2+3 -— **def [RunSteps](/recipes/recipe_modules/bot_update/examples/full.py#25)(api):** +— **def [RunSteps](/recipes/recipe_modules/bot_update/examples/full.py#27)(api):** ### *recipes* / [bot\_update:tests/do\_not\_retry\_patch\_failures\_in\_cq](/recipes/recipe_modules/bot_update/tests/do_not_retry_patch_failures_in_cq.py) [DEPS](/recipes/recipe_modules/bot_update/tests/do_not_retry_patch_failures_in_cq.py#9): [bot\_update](#recipe_modules-bot_update), [gclient](#recipe_modules-gclient), [recipe\_engine/buildbucket][recipe_engine/recipe_modules/buildbucket], [recipe\_engine/cq][recipe_engine/recipe_modules/cq], [recipe\_engine/properties][recipe_engine/recipe_modules/properties], [recipe\_engine/step][recipe_engine/recipe_modules/step] diff --git a/recipes/recipe_modules/bot_update/api.py b/recipes/recipe_modules/bot_update/api.py index cd891abe20..4a3cfbd505 100644 --- a/recipes/recipe_modules/bot_update/api.py +++ b/recipes/recipe_modules/bot_update/api.py @@ -306,7 +306,13 @@ class BotUpdateApi(recipe_api.RecipeApi): step_result = f.result raise finally: - if step_result and step_result.json.output: + # The step_result can be missing the json attribute if the build + # is shutting down and the bot_update script is not able to finish + # writing the json output. + # An AttributeError occuring in this finally block swallows any + # StepFailure that may bubble up. + if (step_result and hasattr(step_result, 'json') + and step_result.json.output): result = step_result.json.output self._last_returned_properties = result.get('properties', {}) diff --git a/recipes/recipe_modules/bot_update/examples/full.py b/recipes/recipe_modules/bot_update/examples/full.py index 1d8372268c..e3f8c5e5b0 100644 --- a/recipes/recipe_modules/bot_update/examples/full.py +++ b/recipes/recipe_modules/bot_update/examples/full.py @@ -2,6 +2,8 @@ # Use of this source code is governed by a BSD-style license that can be # found in the LICENSE file. +from recipe_engine import post_process + PYTHON_VERSION_COMPATIBILITY = 'PY2+3' DEPS = [ @@ -180,6 +182,11 @@ def GenTests(api): api.properties(fail_patch='download') + api.step_data('bot_update', retcode=87) ) + yield (api.test('tryjob_fail_missing_bot_update_json') + try_build() + + api.override_step_data('bot_update', retcode=1) + + api.post_process(post_process.ResultReasonRE, 'Infra Failure.*') + + api.post_process(post_process.StatusException) + + api.post_process(post_process.DropExpectation)) yield ( api.test('clobber') + api.properties(clobber=1)