From ecd670e9ae7fd38b829e21b6b0b53d6d93f66ae7 Mon Sep 17 00:00:00 2001 From: Felix Moessbauer Date: Thu, 18 May 2023 09:15:06 +0200 Subject: [PATCH] fix: collect exceptions on task errors When async tasks fail, all exceptions need to be collected to not get subsequent exceptions about invalid future states. This is achieved by gathering the task results, instead of just waiting for them. By gathering the results, also user-requested cancellation (e.g. via ctrl-c) works without throwing tons of additional exceptions. Since ac437308 we more likely run into that case, which unvealed the bug. By properly handling the exception, a TaskResultError is returned instead of the underlying CommandExecError. This change is reflected in the corresponding unit test. Signed-off-by: Felix Moessbauer Signed-off-by: Jan Kiszka --- kas/kas.py | 9 ++++++++- kas/libkas.py | 18 ++++++++---------- tests/test_commands.py | 4 ++-- 3 files changed, 18 insertions(+), 13 deletions(-) diff --git a/kas/kas.py b/kas/kas.py index ac6e20f..f55487a 100644 --- a/kas/kas.py +++ b/kas/kas.py @@ -98,7 +98,14 @@ def _atexit_handler(): loop = asyncio.get_event_loop() pending = asyncio.Task.all_tasks(loop) if not loop.is_closed(): - loop.run_until_complete(asyncio.gather(*pending)) + # this code path is observed on older python versions (e.g. 3.6). + # In case the loop is not yet closed, tasks still might throw + # exceptions, but we are not interested in these as they are + # likely due to the cancellation. By that, we simply drop them. + try: + loop.run_until_complete(asyncio.gather(*pending)) + except KasUserError: + pass loop.close() diff --git a/kas/libkas.py b/kas/libkas.py index ead6b72..f9a0734 100644 --- a/kas/libkas.py +++ b/kas/libkas.py @@ -197,11 +197,10 @@ def repos_fetch(repos): tasks.append(asyncio.ensure_future(repo.fetch_async())) loop = asyncio.get_event_loop() - loop.run_until_complete(asyncio.wait(tasks)) - - for task in tasks: - if task.result(): - raise TaskExecError('fetch repos', task.result()) + try: + loop.run_until_complete(asyncio.gather(*tasks)) + except CommandExecError as e: + raise TaskExecError('fetch repos', e.ret_code) def repos_apply_patches(repos): @@ -216,11 +215,10 @@ def repos_apply_patches(repos): tasks.append(asyncio.ensure_future(repo.apply_patches_async())) loop = asyncio.get_event_loop() - loop.run_until_complete(asyncio.wait(tasks)) - - for task in tasks: - if task.result(): - raise TaskExecError('apply patches', task.result()) + try: + loop.run_until_complete(asyncio.gather(*tasks)) + except CommandExecError as e: + raise TaskExecError('apply patches', e.ret_code) def get_build_environ(build_system): diff --git a/tests/test_commands.py b/tests/test_commands.py index 1d51053..bcd5d1b 100644 --- a/tests/test_commands.py +++ b/tests/test_commands.py @@ -28,7 +28,7 @@ import json import yaml import pytest from kas import kas -from kas.kasusererror import CommandExecError +from kas.libkas import TaskExecError def test_for_all_repos(changedir, tmpdir): @@ -67,7 +67,7 @@ def test_invalid_checkout(changedir, tmpdir, capsys): tdir = str(tmpdir / 'test_commands') shutil.copytree('tests/test_commands', tdir) os.chdir(tdir) - with pytest.raises(CommandExecError): + with pytest.raises(TaskExecError): kas.kas(['checkout', 'test-invalid.yml'])