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
This commit is contained in:
Sam Doran 2021-01-19 10:07:36 -05:00 committed by GitHub
parent 30d93995dd
commit e8d4b62b41
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
3 changed files with 54 additions and 8 deletions

View file

@ -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)

View file

@ -100,7 +100,21 @@ class AnsibleError(Exception):
with open(file_name, 'r') as f: with open(file_name, 'r') as f:
lines = f.readlines() 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] target_line = lines[line_number]
while not target_line.strip():
line_number -= 1
target_line = lines[line_number]
if line_number > 0: if line_number > 0:
prev_line = lines[line_number - 1] prev_line = lines[line_number - 1]

View file

@ -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") "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 with patch('ansible.errors.to_text', side_effect=IndexError('Raised intentionally')):
self.obj.ansible_pos = ('foo.yml', 2, 1) # raise an IndexError
e = AnsibleError(self.message, self.obj) self.obj.ansible_pos = ('foo.yml', 2, 1)
self.assertEqual( e = AnsibleError(self.message, self.obj)
e.message, self.assertEqual(
("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 " e.message,
"the exact syntax problem.\n\n(specified line no longer in file, maybe it changed?)") ("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 = mock_open()
m.return_value.readlines.return_value = ['this line has unicode \xf0\x9f\x98\xa8 in it!\n'] 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^ " "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") "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")
)