From 9b891b6bb28c9ba976dae196a1a72d751cf936be Mon Sep 17 00:00:00 2001 From: Xavier Morel Date: Thu, 6 Oct 2022 08:42:01 +0000 Subject: [PATCH] [IMP] core: error reporting on tour timeouts When 2e8647bf1662168e5f74395c7c624ef74670c0b2 converted the browser runner to a more reactive / evented system, one bit was missed in "wait_code_ok": concurrent.futures.Future raises exceptions on various events, such as tour timeouts. Because those exceptions were not caught (or just ignored) the code which takes screenshots was bypassed, leading to a lack of screenshots on tour timeouts (and a few other rarer errors), making debugging more complicated. The error reporting was also not ideal as `wait_code_ok` would raise an unexpected (by its caller) `TimeoutError` rather than `ChromeBrowserException`. Fix those two issues, should hopefully makes these occurrences clearer and easier to diagnose. closes odoo/odoo#102403 X-original-commit: 974217968ea970330946c5184bd3b3550ec3cde3 Signed-off-by: Xavier Morel (xmo) --- odoo/tests/common.py | 25 +++++++++++++++++++------ 1 file changed, 19 insertions(+), 6 deletions(-) diff --git a/odoo/tests/common.py b/odoo/tests/common.py index 4002f07fbab..b743f840ce1 100644 --- a/odoo/tests/common.py +++ b/odoo/tests/common.py @@ -1194,12 +1194,13 @@ class ChromeBrowser: self._logger.debug('\n<- %s', msg) except websocket.WebSocketTimeoutException: continue - except Exception: + except Exception as e: # if the socket is still connected something bad happened, # otherwise the client was just shut down - self._result.cancel() if self.ws.connected: + self._result.set_exception(e) raise + self._result.cancel() return res = json.loads(msg) @@ -1486,15 +1487,27 @@ which leads to stray network requests and inconsistencies.""")) }, timeout=timeout)['result'] if res.get('subtype') == 'error': raise ChromeBrowserException("Running code returned an error: %s" % res) - # if the runcode was a promise which took some time to execute, discount - # that from the timeout - if self._result.result(time.time() - start + timeout) and not self.had_failure: + + err = ChromeBrowserException("failed") + try: + # if the runcode was a promise which took some time to execute, + # discount that from the timeout + if self._result.result(time.time() - start + timeout) and not self.had_failure: + return + except CancelledError: + # regular-ish shutdown return + except Exception as e: + err = e self.take_screenshot() self._save_screencast() - raise ChromeBrowserException('Script timeout exceeded') + if isinstance(err, ChromeBrowserException): + raise err + if isinstance(err, concurrent.futures.TimeoutError): + raise ChromeBrowserException('Script timeout exceeded') from err + raise ChromeBrowserException("Unknown error") from err def navigate_to(self, url, wait_stop=False): self._logger.info('Navigating to: "%s"', url)