Skip to content

Animation error handling (Fixes #6416) #6441

New issue

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

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

Already on GitHub? Sign in to your account

Merged
merged 4 commits into from
May 17, 2016

Conversation

dopplershift
Copy link
Contributor

This addresses the remaining issues from #6416

  • raise an error when given arguments that would be ignored (because they are only used to create new instances of MovieWriter)
  • Try to improve exception message when saving movie fails.

The latter can only be improved so much, because often the observed behavior is a BrokenPipeError, which keeps us from being able to grab anything from stderr. My solution here is to note that running with --verbose-debug may be helpful since in this case output is dumped straight to stdout.

When passed arguments that are only used when creating a new MovieWriter
instance, raise an error if writer is an existing instance rather than
silently ignoring the arguments. Also document the fact that these
arguments are invalid in that case.
@dopplershift dopplershift added this to the 1.5.2 (Critical bug fix release) milestone May 17, 2016
raise
raise IOError('Error saving animation to file. '
'Stdout: {0} StdError: {1}. It may help to re-run '
'with --verbose-debug.'.format(out, err))
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The original exception is now getting swallowed. Would probably be nice to at least have it logged through verbose.report()?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think it's even better to just include it in the new exception message.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants