Include error messages returned by ffprobe or ffmpeg in non-zero stat…

…us exception (kokorin#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 * kokorin#273 Ignore failing tests in ffmpeg/ffprobe 5.0 * Renaming to JaffreeAbnormalExitException (cherry picked from commit bdd741c)
marklassau · Feb 8, 2023 · a9b4dc8 · a9b4dc8
1 parent 1e7d1bf
commit a9b4dc8
Show file tree

Hide file tree

Showing 9 changed files with 139 additions and 19 deletions.
diff --git a/src/main/java/com/github/kokorin/jaffree/ffprobe/FFprobeResultReader.java b/src/main/java/com/github/kokorin/jaffree/ffprobe/FFprobeResultReader.java
@@ -19,9 +19,12 @@
 
 import com.github.kokorin.jaffree.ffprobe.data.FormatParser;
 import com.github.kokorin.jaffree.ffprobe.data.ProbeData;
+import com.github.kokorin.jaffree.log.LogMessage;
 import com.github.kokorin.jaffree.process.StdReader;
 
 import java.io.InputStream;
+import java.util.Collections;
+import java.util.List;
 
 /**
  * {@link FFprobeResultReader} adapts {@link StdReader} to {@link FormatParser}.
@@ -48,4 +51,12 @@ public FFprobeResult read(final InputStream stdOut) {
 
         return new FFprobeResult(probeData);
     }
+
+    /**
+     * {@inheritDoc}
+     */
+    @Override
+    public List<LogMessage> getErrorLogMessages() {
+        return Collections.emptyList();
+    }
 }
diff --git a/src/main/java/com/github/kokorin/jaffree/process/BaseStdReader.java b/src/main/java/com/github/kokorin/jaffree/process/BaseStdReader.java
@@ -28,6 +28,8 @@
 import java.io.BufferedReader;
 import java.io.InputStream;
 import java.io.InputStreamReader;
+import java.util.ArrayList;
+import java.util.List;
 
 /**
  * {@link BaseStdReader} reads std output, parses result and logs, and sends logs
@@ -38,6 +40,8 @@
 public abstract class BaseStdReader<T> implements StdReader<T> {
     private static final Logger LOGGER = LoggerFactory.getLogger(BaseStdReader.class);
 
+    private final List<LogMessage> errorLogMessages = new ArrayList<>();
+
     /**
      * Reads provided {@link InputStream} until it's depleted.
      * <p>
@@ -83,6 +87,7 @@ public T read(final InputStream stdOut) {
                     case FATAL:
                     case PANIC:
                     case QUIET:
+                        errorLogMessages.add(logMessage);
                         LOGGER.error(logMessage.message);
                         break;
                 }
@@ -99,6 +104,13 @@ public T read(final InputStream stdOut) {
         return result;
     }
 
+    /**
+     * {@inheritDoc}
+     */
+    public List<LogMessage> getErrorLogMessages() {
+        return errorLogMessages;
+    }
+
     /**
      * @return default result
      */

diff --git a/src/main/java/com/github/kokorin/jaffree/process/GobblingStdReader.java b/src/main/java/com/github/kokorin/jaffree/process/GobblingStdReader.java
@@ -18,19 +18,21 @@
 package com.github.kokorin.jaffree.process;
 
 import com.github.kokorin.jaffree.JaffreeException;
+import com.github.kokorin.jaffree.log.LogMessage;
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
 
 import java.io.IOException;
 import java.io.InputStream;
+import java.util.Collections;
+import java.util.List;
 
 /**
  * {@link StdReader} implementation which reads and ignores bytes read.
  *
  * @param <T> type of parsed result
  */
 public class GobblingStdReader<T> implements StdReader<T> {
-
     private static final Logger LOGGER = LoggerFactory.getLogger(GobblingStdReader.class);
     private static final long REPORT_EVERY_BYTES = 1_000_000;
     private static final int BUFFER_SIZE = 1014;
@@ -65,4 +67,12 @@ public T read(final InputStream stdOut) {
 
         return null;
     }
+
+    /**
+     * {@inheritDoc}
+     */
+    @Override
+    public List<LogMessage> getErrorLogMessages() {
+        return Collections.emptyList();
+    }
 }
diff --git a/src/main/java/com/github/kokorin/jaffree/process/JaffreeAbnormalExitException.java b/src/main/java/com/github/kokorin/jaffree/process/JaffreeAbnormalExitException.java
@@ -0,0 +1,53 @@
+/*
+ *    Copyright 2022 Jon Frydensbjerg
+ *
+ *    Licensed under the Apache License, Version 2.0 (the "License");
+ *    you may not use this file except in compliance with the License.
+ *    You may obtain a copy of the License at
+ *
+ *        http://www.apache.org/licenses/LICENSE-2.0
+ *
+ *    Unless required by applicable law or agreed to in writing, software
+ *    distributed under the License is distributed on an "AS IS" BASIS,
+ *    WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ *    See the License for the specific language governing permissions and
+ *    limitations under the License.
+ *
+ */
+
+package com.github.kokorin.jaffree.process;
+
+import com.github.kokorin.jaffree.JaffreeException;
+import com.github.kokorin.jaffree.log.LogMessage;
+
+import java.util.List;
+
+/**
+ * Non-zero status code exit exception which includes all error messages produced by the process.
+ */
+public class JaffreeAbnormalExitException extends JaffreeException {
+    private List<LogMessage> processErrorLogMessages;
+
+    /**
+     * Constructs a new {@link JaffreeAbnormalExitException} with the specified detail message
+     * and additional context.
+     *
+     * @param message message
+     * @param processErrorLogMessages error log messages produced by the process
+     */
+    public JaffreeAbnormalExitException(final String message,
+                                        final List<LogMessage> processErrorLogMessages) {
+        super(message);
+
+        this.processErrorLogMessages = processErrorLogMessages;
+    }
+
+    /**
+     * Return the list of error log messages.
+     *
+     * @return error log messages
+     */
+    public List<LogMessage> getProcessErrorLogMessages() {
+        return processErrorLogMessages;
+    }
+}
diff --git a/src/main/java/com/github/kokorin/jaffree/process/LoggingStdReader.java b/src/main/java/com/github/kokorin/jaffree/process/LoggingStdReader.java
@@ -18,13 +18,16 @@
 package com.github.kokorin.jaffree.process;
 
 import com.github.kokorin.jaffree.JaffreeException;
+import com.github.kokorin.jaffree.log.LogMessage;
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
 
 import java.io.BufferedReader;
 import java.io.IOException;
 import java.io.InputStream;
 import java.io.InputStreamReader;
+import java.util.Collections;
+import java.util.List;
 
 /**
  * {@link StdReader} implementation which reads and logs everything been read.
@@ -53,4 +56,12 @@ public T read(final InputStream stdOut) {
 
         return null;
     }
+
+    /**
+     * {@inheritDoc}
+     */
+    @Override
+    public List<LogMessage> getErrorLogMessages() {
+        return Collections.emptyList();
+    }
 }
diff --git a/src/main/java/com/github/kokorin/jaffree/process/ProcessHandler.java b/src/main/java/com/github/kokorin/jaffree/process/ProcessHandler.java
@@ -199,10 +199,10 @@ protected T interactWithProcess(final Process process) {
         }
 
         if (!Integer.valueOf(0).equals(status)) {
-            throw new JaffreeException(
-                    "Process execution has ended with non-zero status: " + status
-                            + ". Check logs for detailed error message."
-            );
+            throw new JaffreeAbnormalExitException(
+                "Process execution has ended with non-zero status: " + status
+                    + ". Check logs for detailed error message.",
+                stdErrReader.getErrorLogMessages());
         }
 
         T result = resultRef.get();

diff --git a/src/main/java/com/github/kokorin/jaffree/process/StdReader.java b/src/main/java/com/github/kokorin/jaffree/process/StdReader.java
@@ -17,7 +17,10 @@
 
 package com.github.kokorin.jaffree.process;
 
+import com.github.kokorin.jaffree.log.LogMessage;
+
 import java.io.InputStream;
+import java.util.List;
 
 /**
  * Implement {@link StdReader} interface to parse program stdout or stderr streams.
@@ -32,4 +35,11 @@ public interface StdReader<T> {
      * @return parsed result
      */
     T read(InputStream stdOut);
+
+    /**
+     * Get the list of error messages produced by the running process.
+     *
+     * @return error messages
+     */
+    List<LogMessage> getErrorLogMessages();
 }
diff --git a/src/test/java/com/github/kokorin/jaffree/ffmpeg/FFmpegTest.java b/src/test/java/com/github/kokorin/jaffree/ffmpeg/FFmpegTest.java
@@ -8,8 +8,8 @@
 import com.github.kokorin.jaffree.ffprobe.FFprobe;
 import com.github.kokorin.jaffree.ffprobe.FFprobeResult;
 import com.github.kokorin.jaffree.ffprobe.Stream;
-import com.github.kokorin.jaffree.process.ProcessHandler;
 import com.github.kokorin.jaffree.process.ProcessHelper;
+import com.github.kokorin.jaffree.process.JaffreeAbnormalExitException;
 import org.hamcrest.core.AllOf;
 import org.hamcrest.core.StringContains;
 import org.junit.Assert;
@@ -44,6 +44,7 @@
 import static org.junit.Assert.assertEquals;
 import static org.junit.Assert.assertNull;
 import static org.junit.Assert.assertTrue;
+import static org.junit.Assert.fail;
 
 public class FFmpegTest {
     public static Path ERROR_MP4 = Paths.get("non_existent.mp4");
@@ -496,14 +497,20 @@ public void testMap() throws Exception {
 
     @Test
     public void testExceptionIsThrownIfFfmpegExitsWithError() {
-        expectedException.expect(
-                new StackTraceMatcher("Process execution has ended with non-zero status")
-        );
-
-        FFmpegResult result = FFmpeg.atPath(Config.FFMPEG_BIN)
+        try {
+            FFmpeg.atPath(Config.FFMPEG_BIN)
                 .addInput(UrlInput.fromPath(ERROR_MP4))
                 .addOutput(new NullOutput())
                 .execute();
+        } catch (JaffreeAbnormalExitException e) {
+            assertEquals("Process execution has ended with non-zero status: 1. Check logs for detailed error message.", e.getMessage());
+            assertEquals(1, e.getProcessErrorLogMessages().size());
+            assertEquals("[error] non_existent.mp4: No such file or directory", e.getProcessErrorLogMessages().get(0).message);
+            return;
+        }
+
+        fail("JaffreeAbnormalExitException should have been thrown!");
+
     }
 
     @Test

diff --git a/src/test/java/com/github/kokorin/jaffree/ffprobe/FFprobeTest.java b/src/test/java/com/github/kokorin/jaffree/ffprobe/FFprobeTest.java
@@ -9,8 +9,8 @@
 import com.github.kokorin.jaffree.ffprobe.data.FlatFormatParser;
 import com.github.kokorin.jaffree.ffprobe.data.FormatParser;
 import com.github.kokorin.jaffree.ffprobe.data.JsonFormatParser;
+import com.github.kokorin.jaffree.process.JaffreeAbnormalExitException;
 import org.junit.Assert;
-import org.junit.BeforeClass;
 import org.junit.Ignore;
 import org.junit.Rule;
 import org.junit.Test;
@@ -21,7 +21,6 @@
 import java.io.InputStream;
 import java.nio.channels.SeekableByteChannel;
 import java.nio.file.Files;
-import java.nio.file.Path;
 import java.nio.file.Paths;
 import java.nio.file.StandardOpenOption;
 import java.util.Arrays;
@@ -201,6 +200,7 @@ public void testShowEntries() throws Exception {
     //private boolean showFrames;
 
     @Test
+    @Ignore("fails when run against ffmpeg/ffprobe 5.0")
     public void testShowFrames() throws Exception {
         FFprobeResult result = FFprobe.atPath(Config.FFMPEG_BIN)
                 .setInput(Artifacts.VIDEO_WITH_SUBTITLES)
@@ -383,6 +383,7 @@ public void testSelectStreamWithShowPackets() throws Exception {
     //private boolean showPrograms;
 
     @Test
+    @Ignore("fails when run against ffmpeg/ffprobe 5.0")
     public void testShowPrograms() throws Exception {
         FFprobeResult result = FFprobe.atPath(Config.FFMPEG_BIN)
                 .setInput(Artifacts.VIDEO_WITH_PROGRAMS)
@@ -499,6 +500,7 @@ public void testReadIntervals() throws Exception {
     }
 
     @Test
+    @Ignore("fails when run against ffmpeg/ffprobe 5.0")
     public void testShowPacketsAndFrames() {
         FFprobeResult result = FFprobe.atPath(Config.FFMPEG_BIN)
                 .setInput(Artifacts.VIDEO_WITH_SUBTITLES)
@@ -684,16 +686,20 @@ public void testPacketSideDataListAttributes() throws Exception {
 
     @Test
     public void testExceptionIsThrownIfFfprobeExitsWithError() {
-        expectedException.expect(
-                new StackTraceMatcher("Process execution has ended with non-zero status")
-        );
-
-        FFprobeResult result = FFprobe.atPath(Config.FFMPEG_BIN)
+        try {
+            FFprobe.atPath(Config.FFMPEG_BIN)
                 .setInput(Paths.get("nonexistent.mp4"))
                 .setFormatParser(formatParser)
                 .execute();
-    }
+        } catch (JaffreeAbnormalExitException e) {
+            assertEquals("Process execution has ended with non-zero status: 1. Check logs for detailed error message.", e.getMessage());
+            assertEquals(1, e.getProcessErrorLogMessages().size());
+            assertEquals("[error] nonexistent.mp4: No such file or directory", e.getProcessErrorLogMessages().get(0).message);
+            return;
+        }
 
+        fail("JaffreeAbnormalExitException should have been thrown!");
+    }
 
     @Test
     public void testProbeSize() throws Exception {