Report all swarming and isolate fatal errors in a consistent way.
It makes them easier to extract with regular expressions in log parsing tools.
BUG=309661
Review URL: https://codereview.chromium.org/51383003
git-svn-id: svn://svn.chromium.org/chrome/trunk/tools/swarm_client@231704 0039d316-1c4b-4281-b951-d872f2087c98
diff --git a/isolate.py b/isolate.py
index 9ab79a8..73db3c1 100755
--- a/isolate.py
+++ b/isolate.py
@@ -2449,13 +2449,8 @@
dispatcher = subcommand.CommandDispatcher(__name__)
try:
return dispatcher.execute(OptionParserIsolate(version=__version__), argv)
- except (
- ExecutionError,
- isolateserver.ConfigError,
- isolateserver.MappingError) as e:
- sys.stderr.write('\nError: ')
- sys.stderr.write(str(e))
- sys.stderr.write('\n')
+ except Exception as e:
+ tools.report_error(e)
return 1
diff --git a/isolateserver.py b/isolateserver.py
index d2ad16b..93575a4 100755
--- a/isolateserver.py
+++ b/isolateserver.py
@@ -1652,10 +1652,8 @@
try:
return dispatcher.execute(
OptionParserIsolateServer(version=__version__), args)
- except (ConfigError, MappingError) as e:
- sys.stderr.write('\nError: ')
- sys.stderr.write(str(e))
- sys.stderr.write('\n')
+ except Exception as e:
+ tools.report_error(e)
return 1
diff --git a/run_isolated.py b/run_isolated.py
index dab90b3..54f36d4 100755
--- a/run_isolated.py
+++ b/run_isolated.py
@@ -497,7 +497,7 @@
os_flavor=get_flavor(),
require_command=True)
except isolateserver.ConfigError as e:
- print >> sys.stderr, str(e)
+ tools.report_error(e)
return 1
if settings.read_only:
@@ -517,7 +517,7 @@
with tools.Profiler('RunTest'):
return subprocess.call(settings.command, cwd=cwd, env=env)
except OSError:
- print >> sys.stderr, 'Failed to run %s; cwd=%s' % (settings.command, cwd)
+ tools.report_error('Failed to run %s; cwd=%s' % (settings.command, cwd))
return 1
finally:
if outdir:
@@ -602,6 +602,7 @@
options.isolated or options.hash, storage, cache, algo, outdir)
except Exception as e:
# Make sure any exception is logged.
+ tools.report_error(e)
logging.exception(e)
return 1
diff --git a/swarming.py b/swarming.py
index cc30dc5..4c8f8f0 100755
--- a/swarming.py
+++ b/swarming.py
@@ -133,7 +133,7 @@
uploaded = self.storage.upload_items([self._isolate_item])
elapsed = time.time() - start_time
except (IOError, OSError) as exc:
- print >> sys.stderr, 'Failed to upload the zip file: %s' % exc
+ tools.report_error('Failed to upload the zip file: %s' % exc)
return False
if self._isolate_item in uploaded:
@@ -349,12 +349,12 @@
file_hash = archive(
file_hash_or_isolated, isolate_server, slave_os, algo, verbose)
if not file_hash:
- print >> sys.stderr, 'Archival failure %s' % file_hash_or_isolated
+ tools.report_error('Archival failure %s' % file_hash_or_isolated)
return 1
elif isolateserver.is_valid_hash(file_hash_or_isolated, algo):
file_hash = file_hash_or_isolated
else:
- print >> sys.stderr, 'Invalid hash %s' % file_hash_or_isolated
+ tools.report_error('Invalid hash %s' % file_hash_or_isolated)
return 1
try:
@@ -371,7 +371,7 @@
priority,
algo)
except ValueError as e:
- print >> sys.stderr, 'Unable to process %s: %s' % (test_name, e)
+ tools.report_error('Unable to process %s: %s' % (test_name, e))
return 1
chromium_setup(manifest)
@@ -389,16 +389,18 @@
manifest_text = manifest.to_json()
result = net.url_read(test_url, data={'request': manifest_text})
if not result:
- print >> sys.stderr, 'Failed to send test for %s\n%s' % (
- test_name, test_url)
+ tools.report_error(
+ 'Failed to send test for %s\n%s' % (test_name, test_url))
return 1
try:
json.loads(result)
except (ValueError, TypeError) as e:
- print >> sys.stderr, 'Failed to send test for %s' % test_name
- print >> sys.stderr, 'Manifest: %s' % manifest_text
- print >> sys.stderr, 'Bad response: %s' % result
- print >> sys.stderr, str(e)
+ msg = '\n'.join((
+ 'Failed to send test for %s' % test_name,
+ 'Manifest: %s' % manifest_text,
+ 'Bad response: %s' % result,
+ str(e)))
+ tools.report_error(msg)
return 1
return 0
@@ -540,7 +542,8 @@
try:
return collect(options.swarming, args[0], options.timeout, options.decorate)
except Failure as e:
- parser.error(e.args[0])
+ tools.report_error(e)
+ return 1
@subcommand.usage('[hash|isolated ...]')
@@ -574,12 +577,12 @@
except Failure as e:
result = e.args[0]
if result:
- print >> sys.stderr, 'Failed to trigger %s: %s' % (arg, result)
+ tools.report_error('Failed to trigger %s: %s' % (arg, result))
else:
success.append(os.path.basename(arg))
if not success:
- print >> sys.stderr, 'Failed to trigger any job.'
+ tools.report_error('Failed to trigger any job.')
return result
code = 0
@@ -594,7 +597,7 @@
code = max(code, new_code)
except Failure as e:
code = max(code, 1)
- print >> sys.stderr, e.args[0]
+ tools.report_error(e)
return code
@@ -632,7 +635,8 @@
options.profile,
options.priority)
except Failure as e:
- parser.error(e.args[0])
+ tools.report_error(e)
+ return 1
class OptionParserSwarming(tools.OptionParserWithLogging):
@@ -657,10 +661,8 @@
dispatcher = subcommand.CommandDispatcher(__name__)
try:
return dispatcher.execute(OptionParserSwarming(version=__version__), args)
- except Failure as e:
- sys.stderr.write('\nError: ')
- sys.stderr.write(str(e))
- sys.stderr.write('\n')
+ except Exception as e:
+ tools.report_error(e)
return 1
diff --git a/tests/isolate_smoke_test.py b/tests/isolate_smoke_test.py
index d2e66e8..9adab06 100755
--- a/tests/isolate_smoke_test.py
+++ b/tests/isolate_smoke_test.py
@@ -364,11 +364,10 @@
self._expect_no_result()
root = file_path.get_native_path_case(unicode(ROOT_DIR))
expected = (
- '\n'
- 'Error: Input directory %s must have a trailing slash\n' %
+ 'Input directory %s must have a trailing slash' %
os.path.join(root, 'tests', 'isolate', 'files1')
)
- self.assertEqual(expected, out)
+ self.assertIn(expected, out)
def _test_non_existent(self, mode):
try:
@@ -381,11 +380,10 @@
self._expect_no_result()
root = file_path.get_native_path_case(unicode(ROOT_DIR))
expected = (
- '\n'
- 'Error: Input file %s doesn\'t exist\n' %
+ 'Input file %s doesn\'t exist' %
os.path.join(root, 'tests', 'isolate', 'A_file_that_do_not_exist')
)
- self.assertEqual(expected, out)
+ self.assertIn(expected, out)
def _test_all_items_invalid(self, mode):
out = self._execute(mode, 'all_items_invalid.isolate',
@@ -842,9 +840,9 @@
err = e.stderr
self._expect_no_tree()
self._expect_no_result()
- expected = '\nError: No command to run.\n'
+ expected = 'No command to run.'
self.assertEqual('', out)
- self.assertEqual(expected, err)
+ self.assertIn(expected, err)
# TODO(csharp): Disabled until crbug.com/150823 is fixed.
def do_not_test_touch_only(self):
diff --git a/tests/run_isolated_smoke_test.py b/tests/run_isolated_smoke_test.py
index 45839b4..49f6984 100755
--- a/tests/run_isolated_smoke_test.py
+++ b/tests/run_isolated_smoke_test.py
@@ -187,7 +187,7 @@
out, err, returncode = self._run(self._generate_args_with_hash(result_hash))
if not VERBOSE:
self.assertEqual('', out)
- self.assertEqual('No command to run\n', err)
+ self.assertIn('No command to run\n', err)
self.assertEqual(1, returncode)
actual = list_files_tree(self.cache)
self.assertEqual(sorted(expected), actual)
diff --git a/utils/tools.py b/utils/tools.py
index eecf709..2885954 100644
--- a/utils/tools.py
+++ b/utils/tools.py
@@ -10,6 +10,7 @@
import os
import sys
import time
+import traceback
class OptionParserWithLogging(optparse.OptionParser):
@@ -112,3 +113,23 @@
elif out[0].endswith('.py'):
out.insert(0, sys.executable)
return out
+
+
+def report_error(error):
+ """Prints a error to stderr, wrapping it into header and footer.
+
+ That way errors can be reliably extracted from logs. It's indented to be used
+ only for non recoverable unexpected errors. Is should NOT be used for input
+ validation, command line argument errors, etc.
+
+ Arguments:
+ error: error message string (possibly multiple lines) or an instance of
+ Exception subclass. In the later case a traceback will also be
+ reported. It's assumed that |report_error| is called in an except
+ block where |error| was caught.
+ """
+ print >> sys.stderr, '[------ Swarming Error ------]'
+ print >> sys.stderr, str(error)
+ if isinstance(error, Exception):
+ print >> sys.stderr, traceback.format_exc(),
+ print >> sys.stderr, '[----------------------------]'
