Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Handle empty file in get_metadata correctly #568

Merged
merged 1 commit into from
Nov 25, 2019
Merged

Conversation

Mofef
Copy link
Contributor

@Mofef Mofef commented Sep 16, 2019

If the metadata_file_path exists but the metadata_file is empty, yaml.safe_load(metadata_file) will return None. But the caller expects a dictionary which can lead to a crash. (e.g. in https://github.com/catkin/catkin_tools/blob/master/catkin_tools/verbs/catkin_build/cli.py#L371)

I'm not sure how I ended up with this file being empty (I can't remember touching it)...

If the `metadata_file_path` exists but the `metadata_file` is empty, `yaml.safe_load(metadata_file)` will return None. But the caller expects a dictionary which can lead to a crash. (e.g. in https://github.com/catkin/catkin_tools/blob/master/catkin_tools/verbs/catkin_build/cli.py#L371)
@mikepurvis
Copy link
Member

Fine with this change for now, though perhaps a better approach if others hit this would be to catch yaml errors and emit a warning.

@mikepurvis mikepurvis merged commit a5dfdfe into catkin:master Nov 25, 2019
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants