Skip to content

Commit

Permalink
Fix unicode handling in Exiters (#6032)
Browse files Browse the repository at this point in the history
### Problem

Three unicode issues had snuck into the two `Exiter` implementations (with/without `pantsd`), which prevented a useful error from rendering in the local case, and caused `pantsd` to hang up in the remote case.

### Solution

Add a unicode-containing compilation failure integration test that runs with/without `pantsd`, and fix the three issues.

### Result

Unicode-containing exceptions should render correctly in more cases.
  • Loading branch information
Stu Hood authored Jun 27, 2018
1 parent c5e0102 commit 8fdee38
Show file tree
Hide file tree
Showing 7 changed files with 43 additions and 5 deletions.
2 changes: 1 addition & 1 deletion build-support/bin/pre-commit.sh
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ $(git rev-parse --verify master > /dev/null 2>&1)
if [[ $? -eq 0 ]]; then
echo "* Checking imports" && ./build-support/bin/isort.sh || \
die "To fix import sort order, run \`\"$(pwd)/build-support/bin/isort.sh\" -f\`"
echo "* Checking lint" && ./pants -q --changed-parent=master lint || exit 1
echo "* Checking lint" && ./pants -q --exclude-target-regexp='testprojects/.*' --changed-parent=master lint || exit 1
else
# When travis builds a tag, it does so in a shallow clone without master fetched, which
# fails in pants changed.
Expand Down
7 changes: 4 additions & 3 deletions src/python/pants/base/exiter.py
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@
import traceback

import faulthandler
import six

from pants.util.dirutil import safe_open

Expand Down Expand Up @@ -71,7 +72,7 @@ def exit(self, result=0, msg=None, out=None):
:param out: The file descriptor to emit `msg` to. (Optional)
"""
if msg:
print(msg, file=out or sys.stderr)
print(msg.encode('utf-8'), file=out or sys.stderr)
self._exit(result)

def exit_and_fail(self, msg=None):
Expand All @@ -90,7 +91,7 @@ def handle_unhandled_exception(self, exc_class=None, exc=None, tb=None, add_newl
def format_msg(print_backtrace=True):
msg = 'Exception caught: ({})\n'.format(type(exc))
msg += '{}\n'.format(''.join(self._format_tb(tb))) if print_backtrace else '\n'
msg += 'Exception message: {}\n'.format(exc if str(exc) else 'none')
msg += 'Exception message: {}\n'.format(six.text_type(exc) if exc else 'none')
msg += '\n' if add_newline else ''
return msg

Expand All @@ -106,7 +107,7 @@ def _log_exception(self, msg):
exception_log.write('timestamp: {}\n'.format(datetime.datetime.now().isoformat()))
exception_log.write('args: {}\n'.format(sys.argv))
exception_log.write('pid: {}\n'.format(os.getpid()))
exception_log.write(msg)
exception_log.write(msg.encode('utf-8'))
exception_log.write('\n')
except Exception as e:
# This is all error recovery logic so we catch all exceptions from the logic above because
Expand Down
1 change: 1 addition & 0 deletions testprojects/src/python/unicode/compilation_failure/BUILD
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
python_library()
4 changes: 4 additions & 0 deletions testprojects/src/python/unicode/compilation_failure/main.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
# This file is expected to fail to "compile", and raise a unicode error while doing so.
# Because the error itself contains unicode, it can exercise that error handling codepaths
# are unicode aware.
import sys¡
10 changes: 10 additions & 0 deletions tests/python/pants_test/base/BUILD
Original file line number Diff line number Diff line change
Expand Up @@ -207,3 +207,13 @@ python_tests(
'tests/python/pants_test/option:testing',
],
)

python_tests(
name = 'exiter_integration',
sources = [ 'test_exiter_integration.py' ],
dependencies = [
'tests/python/pants_test:int-test',
],
tags = {'integration'},
timeout = 120,
)
21 changes: 21 additions & 0 deletions tests/python/pants_test/base/test_exiter_integration.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,21 @@
# coding=utf-8
# Copyright 2018 Pants project contributors (see CONTRIBUTORS.md).
# Licensed under the Apache License, Version 2.0 (see LICENSE).

from __future__ import (absolute_import, division, generators, nested_scopes, print_function,
unicode_literals, with_statement)

from pants_test.pants_run_integration_test import PantsRunIntegrationTest, ensure_daemon


class ExiterIntegrationTest(PantsRunIntegrationTest):
"""Tests that "interesting" exceptions are properly rendered."""

@ensure_daemon
def test_unicode_containing_exception(self):
"""Test whether we can run a single target without special flags."""
pants_run = self.run_pants(['test', 'testprojects/src/python/unicode/compilation_failure'])
self.assert_failure(pants_run)

self.assertIn('during bytecode compilation', pants_run.stderr_data)
self.assertIn('import sys¡', pants_run.stderr_data)
Original file line number Diff line number Diff line change
Expand Up @@ -67,7 +67,8 @@ def targets(self):
'testprojects/tests/java/org/pantsbuild/testproject/depman:old-tests',
'testprojects/tests/java/org/pantsbuild/testproject/htmlreport:htmlreport',
'testprojects/tests/java/org/pantsbuild/testproject/parallel.*',
'testprojects/src/python/python_distribution/fasthello_with_install_requires.*'
'testprojects/src/python/python_distribution/fasthello_with_install_requires.*u',
'testprojects/src/python/unicode/compilation_failure',
]

# May not succeed without java8 installed
Expand Down

0 comments on commit 8fdee38

Please sign in to comment.