_yaml: fix confusing error message#2089
Conversation
This moves the ruamel-yaml CParser import instantiation outside of try-catch. The catch reports 'Severely malformed YAML', which isn't the case if you you cant't import CParser
| cpdef MappingNode load_data(str data, int file_index=node._SYNTHETIC_FILE_INDEX, str file_name=None, bint copy_tree=False): | ||
| cdef Representer rep | ||
|
|
||
| try: |
There was a problem hiding this comment.
Ideally, we would have the precise exception below instead of the blind except Exception statement.
Given that determining what exception specifically is raised there may be quite difficult, I think I'd still be more comfortable with an additional except ImportError block, so that anything else happening inside the yaml.CParser(data) constructor is still in the try block.
E.g. just adding the following before the blind except may be preferrable:
try:
...
except ImportError as e:
raise e
except Exception:
... keeping the same blind except...
``There was a problem hiding this comment.
The error reported in #1833 ("'NoneType' object is not callable") is not an ImportError, so this wouldn't help.
What about something like
try:
rep = Representer(file_index)
parser = yaml.CParser(data)
except Exception as e:
raise LoadError("YAML parser initialization failed:\n\n{}\n\n".format(e),
LoadErrorReason.INVALID_YAML) from e
try:
try:
while parser.check_event():
...
I.e., just switching to a more suitable error message. (We could also define a new LoadErrorReason if we prefer).
|
Link to relevant issue: #1833 |
This moves the ruamel-yaml CParser import instantiation outside of try-catch. The catch reports 'Severely malformed YAML', which isn't the case if you you cant't import CParser