From 75c0b4b71506b116fedc2200daf832365ad35523 Mon Sep 17 00:00:00 2001 From: Andrii Shyshkalov Date: Fri, 17 Nov 2017 17:17:28 -0800 Subject: [PATCH] bot_update: don't crash badly if there no json outpot. To illustrate, with this CL it's clear what is broken: https://ci.chromium.org/swarming/task/39e5676c13f00710?server=chromium-swarm.appspot.com while before you get a red herring: ncaught Exception: AttributeError("'NoneType' object has no attribute 'get'",)... https://ci.chromium.org/swarming/task/39e55d46c0774610?server=chromium-swarm.appspot.com R=iannucci@chromium.org Bug: 786486 Change-Id: I9ed2cdb7261c2e22a5a68116b81f841ece06c5d2 Reviewed-on: https://chromium-review.googlesource.com/777556 Reviewed-by: Robbie Iannucci Commit-Queue: Andrii Shyshkalov --- recipes/README.recipes.md | 4 +- recipes/recipe_modules/bot_update/api.py | 2 +- .../examples/full.expected/tryjob_fail.json | 72 ++----------------- .../bot_update/examples/full.py | 3 +- 4 files changed, 9 insertions(+), 72 deletions(-) diff --git a/recipes/README.recipes.md b/recipes/README.recipes.md index 2905fac7e6..c920e8f882 100644 --- a/recipes/README.recipes.md +++ b/recipes/README.recipes.md @@ -752,9 +752,9 @@ like checkout or compile), and some of these tests have failed. ### *recipes* / [bot\_update:examples/full](/recipes/recipe_modules/bot_update/examples/full.py) -[DEPS](/recipes/recipe_modules/bot_update/examples/full.py#5): [bot\_update](#recipe_modules-bot_update), [gclient](#recipe_modules-gclient), [gerrit](#recipe_modules-gerrit), [recipe\_engine/path][recipe_engine/recipe_modules/path], [recipe\_engine/platform][recipe_engine/recipe_modules/platform], [recipe\_engine/properties][recipe_engine/recipe_modules/properties] +[DEPS](/recipes/recipe_modules/bot_update/examples/full.py#5): [bot\_update](#recipe_modules-bot_update), [gclient](#recipe_modules-gclient), [gerrit](#recipe_modules-gerrit), [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] -— **def [RunSteps](/recipes/recipe_modules/bot_update/examples/full.py#14)(api):** +— **def [RunSteps](/recipes/recipe_modules/bot_update/examples/full.py#15)(api):** ### *recipes* / [cipd:examples/full](/recipes/recipe_modules/cipd/examples/full.py) [DEPS](/recipes/recipe_modules/cipd/examples/full.py#8): [cipd](#recipe_modules-cipd), [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/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 194c15c82b..7454c7e039 100644 --- a/recipes/recipe_modules/bot_update/api.py +++ b/recipes/recipe_modules/bot_update/api.py @@ -283,7 +283,7 @@ class BotUpdateApi(recipe_api.RecipeApi): step_result = f.result raise finally: - if step_result: + if step_result and step_result.json.output: result = step_result.json.output self._last_returned_properties = step_result.json.output.get( 'properties', {}) diff --git a/recipes/recipe_modules/bot_update/examples/full.expected/tryjob_fail.json b/recipes/recipe_modules/bot_update/examples/full.expected/tryjob_fail.json index e6de21f281..ebd20163a4 100644 --- a/recipes/recipe_modules/bot_update/examples/full.expected/tryjob_fail.json +++ b/recipes/recipe_modules/bot_update/examples/full.expected/tryjob_fail.json @@ -35,74 +35,10 @@ "name": "bot_update", "~followup_annotations": [ "step returned non-zero exit code: 1", - "@@@STEP_TEXT@Some step text@@@", - "@@@STEP_LOG_LINE@json.output@{@@@", - "@@@STEP_LOG_LINE@json.output@ \"did_run\": true, @@@", - "@@@STEP_LOG_LINE@json.output@ \"fixed_revisions\": {@@@", - "@@@STEP_LOG_LINE@json.output@ \"src\": \"HEAD\"@@@", - "@@@STEP_LOG_LINE@json.output@ }, @@@", - "@@@STEP_LOG_LINE@json.output@ \"manifest\": {@@@", - "@@@STEP_LOG_LINE@json.output@ \"src\": {@@@", - "@@@STEP_LOG_LINE@json.output@ \"repository\": \"https://fake.org/src.git\", @@@", - "@@@STEP_LOG_LINE@json.output@ \"revision\": \"f27fede2220bcd326aee3e86ddfd4ebd0fe58cb9\"@@@", - "@@@STEP_LOG_LINE@json.output@ }, @@@", - "@@@STEP_LOG_LINE@json.output@ \"src/third_party/angle\": {@@@", - "@@@STEP_LOG_LINE@json.output@ \"repository\": \"https://fake.org/src/third_party/angle.git\", @@@", - "@@@STEP_LOG_LINE@json.output@ \"revision\": \"fac9503c46405f77757b9a728eb85b8d7bc6080c\"@@@", - "@@@STEP_LOG_LINE@json.output@ }, @@@", - "@@@STEP_LOG_LINE@json.output@ \"src/v8\": {@@@", - "@@@STEP_LOG_LINE@json.output@ \"repository\": \"https://fake.org/src/v8.git\", @@@", - "@@@STEP_LOG_LINE@json.output@ \"revision\": \"801ada225ddc271c132c3a35f03975671d43e399\"@@@", - "@@@STEP_LOG_LINE@json.output@ }@@@", - "@@@STEP_LOG_LINE@json.output@ }, @@@", - "@@@STEP_LOG_LINE@json.output@ \"patch_failure\": false, @@@", - "@@@STEP_LOG_LINE@json.output@ \"patch_root\": \"src\", @@@", - "@@@STEP_LOG_LINE@json.output@ \"properties\": {@@@", - "@@@STEP_LOG_LINE@json.output@ \"got_angle_revision\": \"fac9503c46405f77757b9a728eb85b8d7bc6080c\", @@@", - "@@@STEP_LOG_LINE@json.output@ \"got_angle_revision_cp\": \"refs/heads/master@{#297276}\", @@@", - "@@@STEP_LOG_LINE@json.output@ \"got_cr_revision\": \"f27fede2220bcd326aee3e86ddfd4ebd0fe58cb9\", @@@", - "@@@STEP_LOG_LINE@json.output@ \"got_cr_revision_cp\": \"refs/heads/master@{#170242}\", @@@", - "@@@STEP_LOG_LINE@json.output@ \"got_revision\": \"f27fede2220bcd326aee3e86ddfd4ebd0fe58cb9\", @@@", - "@@@STEP_LOG_LINE@json.output@ \"got_revision_cp\": \"refs/heads/master@{#170242}\", @@@", - "@@@STEP_LOG_LINE@json.output@ \"got_v8_revision\": \"801ada225ddc271c132c3a35f03975671d43e399\", @@@", - "@@@STEP_LOG_LINE@json.output@ \"got_v8_revision_cp\": \"refs/heads/master@{#43426}\"@@@", - "@@@STEP_LOG_LINE@json.output@ }, @@@", - "@@@STEP_LOG_LINE@json.output@ \"root\": \"src\", @@@", - "@@@STEP_LOG_LINE@json.output@ \"source_manifest\": {@@@", - "@@@STEP_LOG_LINE@json.output@ \"directories\": {@@@", - "@@@STEP_LOG_LINE@json.output@ \"src\": {@@@", - "@@@STEP_LOG_LINE@json.output@ \"git_checkout\": {@@@", - "@@@STEP_LOG_LINE@json.output@ \"repo_url\": \"https://fake.org/src.git\", @@@", - "@@@STEP_LOG_LINE@json.output@ \"revision\": \"f27fede2220bcd326aee3e86ddfd4ebd0fe58cb9\"@@@", - "@@@STEP_LOG_LINE@json.output@ }@@@", - "@@@STEP_LOG_LINE@json.output@ }, @@@", - "@@@STEP_LOG_LINE@json.output@ \"src/third_party/angle\": {@@@", - "@@@STEP_LOG_LINE@json.output@ \"git_checkout\": {@@@", - "@@@STEP_LOG_LINE@json.output@ \"repo_url\": \"https://fake.org/src/third_party/angle.git\", @@@", - "@@@STEP_LOG_LINE@json.output@ \"revision\": \"fac9503c46405f77757b9a728eb85b8d7bc6080c\"@@@", - "@@@STEP_LOG_LINE@json.output@ }@@@", - "@@@STEP_LOG_LINE@json.output@ }, @@@", - "@@@STEP_LOG_LINE@json.output@ \"src/v8\": {@@@", - "@@@STEP_LOG_LINE@json.output@ \"git_checkout\": {@@@", - "@@@STEP_LOG_LINE@json.output@ \"repo_url\": \"https://fake.org/src/v8.git\", @@@", - "@@@STEP_LOG_LINE@json.output@ \"revision\": \"801ada225ddc271c132c3a35f03975671d43e399\"@@@", - "@@@STEP_LOG_LINE@json.output@ }@@@", - "@@@STEP_LOG_LINE@json.output@ }@@@", - "@@@STEP_LOG_LINE@json.output@ }, @@@", - "@@@STEP_LOG_LINE@json.output@ \"version\": 0@@@", - "@@@STEP_LOG_LINE@json.output@ }, @@@", - "@@@STEP_LOG_LINE@json.output@ \"step_text\": \"Some step text\"@@@", - "@@@STEP_LOG_LINE@json.output@}@@@", - "@@@STEP_LOG_END@json.output@@@", - "@@@STEP_EXCEPTION@@@", - "@@@SET_BUILD_PROPERTY@got_angle_revision@\"fac9503c46405f77757b9a728eb85b8d7bc6080c\"@@@", - "@@@SET_BUILD_PROPERTY@got_angle_revision_cp@\"refs/heads/master@{#297276}\"@@@", - "@@@SET_BUILD_PROPERTY@got_cr_revision@\"f27fede2220bcd326aee3e86ddfd4ebd0fe58cb9\"@@@", - "@@@SET_BUILD_PROPERTY@got_cr_revision_cp@\"refs/heads/master@{#170242}\"@@@", - "@@@SET_BUILD_PROPERTY@got_revision@\"f27fede2220bcd326aee3e86ddfd4ebd0fe58cb9\"@@@", - "@@@SET_BUILD_PROPERTY@got_revision_cp@\"refs/heads/master@{#170242}\"@@@", - "@@@SET_BUILD_PROPERTY@got_v8_revision@\"801ada225ddc271c132c3a35f03975671d43e399\"@@@", - "@@@SET_BUILD_PROPERTY@got_v8_revision_cp@\"refs/heads/master@{#43426}\"@@@" + "@@@STEP_LOG_END@json.output (invalid)@@@", + "@@@STEP_LOG_LINE@json.output (exception)@No JSON object could be decoded@@@", + "@@@STEP_LOG_END@json.output (exception)@@@", + "@@@STEP_EXCEPTION@@@" ] }, { diff --git a/recipes/recipe_modules/bot_update/examples/full.py b/recipes/recipe_modules/bot_update/examples/full.py index d28e650492..a172425835 100644 --- a/recipes/recipe_modules/bot_update/examples/full.py +++ b/recipes/recipe_modules/bot_update/examples/full.py @@ -6,6 +6,7 @@ DEPS = [ 'bot_update', 'gclient', 'gerrit', + 'recipe_engine/json', 'recipe_engine/path', 'recipe_engine/platform', 'recipe_engine/properties', @@ -151,7 +152,7 @@ def GenTests(api): issue=12345, patchset=654321, rietveld='https://rietveld.example.com/', - ) + api.step_data('bot_update', retcode=1) + ) + api.step_data('bot_update', api.json.invalid(None), retcode=1) yield api.test('tryjob_fail_patch') + api.properties( issue=12345, patchset=654321,