Skip to content
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

Include error messages returned by ffprobe or ffmpeg in non-zero status exception #306

Merged
merged 11 commits into from
Jun 3, 2022

Conversation

jonfryd
Copy link
Contributor

@jonfryd jonfryd commented Jun 3, 2022

So basically this does what you suggested to me yesterday, I think.

  1. I had to extend StdReader a bit by adding a getter which returns a list of log messages.
  2. BaseStdReader records any errors found in reverse order (I'm usually interested in the last error returned)
  3. ProcessHandler adds the list of errors formatted in angular brackets

Copy link
Owner

@kokorin kokorin left a comment

Choose a reason for hiding this comment

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

Good start!

@kokorin
Copy link
Owner

kokorin commented Jun 3, 2022

Failed tests are probably related to #273

@jonfryd
Copy link
Contributor Author

jonfryd commented Jun 3, 2022

Good start!

Thanks for the fast review - much appreciated!

@jonfryd
Copy link
Contributor Author

jonfryd commented Jun 3, 2022

Failed tests are probably related to #273

OK, no problem!

@jonfryd
Copy link
Contributor Author

jonfryd commented Jun 3, 2022

I've now addressed your initial review comments, @kokorin

@kokorin
Copy link
Owner

kokorin commented Jun 3, 2022

@jonfryd Please add @Ignore to all tests which fail because of ffmpeg/ffprobe 5.0 version

@jonfryd
Copy link
Contributor Author

jonfryd commented Jun 3, 2022

@jonfryd Please add @Ignore to all tests which fail because of ffmpeg/ffprobe 5.0 version

Done

@jonfryd jonfryd changed the title Include error messages returned by ffprobe or ffmpeg in non-zero status exception message Include error messages returned by ffprobe or ffmpeg in non-zero status exception Jun 3, 2022
@jonfryd
Copy link
Contributor Author

jonfryd commented Jun 3, 2022

Let me know if you need additional changes to this, @kokorin

@jonfryd
Copy link
Contributor Author

jonfryd commented Jun 3, 2022

Thanks for reviewing this, @kokorin 👍 Have a nice weekend!

@kokorin kokorin merged commit bdd741c into kokorin:master Jun 3, 2022
kokorin pushed a commit that referenced this pull request Feb 8, 2023
…us exception (#306)

* Include any error messages returned by ffprobe or ffmpeg in exception message

* Return empty lists

* Record errors in switch/case instead

* Record messages in chronological order instead

* Include process errors in a new exception

* Don't use wilcard import

* Added missing fail import

* Include QUIET, as well

* Don't use underscore style

* #273 Ignore failing tests in ffmpeg/ffprobe 5.0

* Renaming to JaffreeAbnormalExitException

(cherry picked from commit bdd741c)
kokorin pushed a commit that referenced this pull request Feb 8, 2023
…us exception (#306)

* Include any error messages returned by ffprobe or ffmpeg in exception message

* Return empty lists

* Record errors in switch/case instead

* Record messages in chronological order instead

* Include process errors in a new exception

* Don't use wilcard import

* Added missing fail import

* Include QUIET, as well

* Don't use underscore style

* #273 Ignore failing tests in ffmpeg/ffprobe 5.0

* Renaming to JaffreeAbnormalExitException

(cherry picked from commit bdd741c)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants