From e8d4b62b411fb7e3ba3131906a4122185d417266 Mon Sep 17 00:00:00 2001 From: Sam Doran Date: Tue, 19 Jan 2021 10:07:36 -0500 Subject: [PATCH] Fix YAML error message when error is at the end of the file (#73241) * Fix YAML error message when error is at the end of the file If a YAML file fails to load due to a syntax error in a file, or there is an error in the last line of a file, PyYAML reports the last line number of the file as the index where the error occurred. When reading the file lines, we use that index to the get the relevant line. If the index value is out of range, the relevant line is lost for error reporting. Subtract one from the index value to avoid the IndexError in this specific scenario. It is possible to still get an IndexError, which will be handled as it is currently. * Update existing tests and add new tests --- ...ML-error-message-when-file-load-failed.yml | 2 + lib/ansible/errors/__init__.py | 14 ++++++ test/units/errors/test_errors.py | 46 +++++++++++++++---- 3 files changed, 54 insertions(+), 8 deletions(-) create mode 100644 changelogs/fragments/16456-correct-YAML-error-message-when-file-load-failed.yml diff --git a/changelogs/fragments/16456-correct-YAML-error-message-when-file-load-failed.yml b/changelogs/fragments/16456-correct-YAML-error-message-when-file-load-failed.yml new file mode 100644 index 00000000000..df16e7fb351 --- /dev/null +++ b/changelogs/fragments/16456-correct-YAML-error-message-when-file-load-failed.yml @@ -0,0 +1,2 @@ +bugfixes: + - display correct error information when an error exists in the last line of the file (https://github.com/ansible/ansible/issues/16456) diff --git a/lib/ansible/errors/__init__.py b/lib/ansible/errors/__init__.py index c9831a5cdd6..782d0719d16 100644 --- a/lib/ansible/errors/__init__.py +++ b/lib/ansible/errors/__init__.py @@ -100,7 +100,21 @@ class AnsibleError(Exception): with open(file_name, 'r') as f: lines = f.readlines() + # In case of a YAML loading error, PyYAML will report the very last line + # as the location of the error. Avoid an index error here in order to + # return a helpful message. + file_length = len(lines) + if line_number >= file_length: + line_number = file_length - 1 + + # If target_line contains only whitespace, move backwards until + # actual code is found. If there are several empty lines after target_line, + # the error lines would just be blank, which is not very helpful. target_line = lines[line_number] + while not target_line.strip(): + line_number -= 1 + target_line = lines[line_number] + if line_number > 0: prev_line = lines[line_number - 1] diff --git a/test/units/errors/test_errors.py b/test/units/errors/test_errors.py index ab5c19cdcdb..136a2695099 100644 --- a/test/units/errors/test_errors.py +++ b/test/units/errors/test_errors.py @@ -97,14 +97,15 @@ class TestErrors(unittest.TestCase): "the exact syntax problem.\n\nThe offending line appears to be:\n\n\nthis is line 1\n^ here\n") ) - # this line will not be found, as it is out of the index range - self.obj.ansible_pos = ('foo.yml', 2, 1) - e = AnsibleError(self.message, self.obj) - self.assertEqual( - e.message, - ("This is the error message\n\nThe error appears to be in 'foo.yml': line 2, column 1, but may\nbe elsewhere in the file depending on " - "the exact syntax problem.\n\n(specified line no longer in file, maybe it changed?)") - ) + with patch('ansible.errors.to_text', side_effect=IndexError('Raised intentionally')): + # raise an IndexError + self.obj.ansible_pos = ('foo.yml', 2, 1) + e = AnsibleError(self.message, self.obj) + self.assertEqual( + e.message, + ("This is the error message\n\nThe error appears to be in 'foo.yml': line 2, column 1, but may\nbe elsewhere in the file depending on " + "the exact syntax problem.\n\n(specified line no longer in file, maybe it changed?)") + ) m = mock_open() m.return_value.readlines.return_value = ['this line has unicode \xf0\x9f\x98\xa8 in it!\n'] @@ -119,3 +120,32 @@ class TestErrors(unittest.TestCase): "file depending on the exact syntax problem.\n\nThe offending line appears to be:\n\n\nthis line has unicode \xf0\x9f\x98\xa8 in it!\n^ " "here\n") ) + + def test_get_error_lines_error_in_last_line(self): + m = mock_open() + m.return_value.readlines.return_value = ['this is line 1\n', 'this is line 2\n', 'this is line 3\n'] + + with patch('{0}.open'.format(BUILTINS), m): + # If the error occurs in the last line of the file, use the correct index to get the line + # and avoid the IndexError + self.obj.ansible_pos = ('foo.yml', 4, 1) + e = AnsibleError(self.message, self.obj) + self.assertEqual( + e.message, + ("This is the error message\n\nThe error appears to be in 'foo.yml': line 4, column 1, but may\nbe elsewhere in the file depending on " + "the exact syntax problem.\n\nThe offending line appears to be:\n\nthis is line 2\nthis is line 3\n^ here\n") + ) + + def test_get_error_lines_error_empty_lines_around_error(self): + """Test that trailing whitespace after the error is removed""" + m = mock_open() + m.return_value.readlines.return_value = ['this is line 1\n', 'this is line 2\n', 'this is line 3\n', ' \n', ' \n', ' '] + + with patch('{0}.open'.format(BUILTINS), m): + self.obj.ansible_pos = ('foo.yml', 5, 1) + e = AnsibleError(self.message, self.obj) + self.assertEqual( + e.message, + ("This is the error message\n\nThe error appears to be in 'foo.yml': line 5, column 1, but may\nbe elsewhere in the file depending on " + "the exact syntax problem.\n\nThe offending line appears to be:\n\nthis is line 2\nthis is line 3\n^ here\n") + )