FrameFile: Improve error messages. (apache#16912)

* FrameFile: Improve error messages. 1) Include frame file path in error messages. 2) Adhere better to style (no space before brackets). * Fix test.
317brian · Aug 20, 2024 · 2bd3160 · 2bd3160
1 parent 7b8573e
commit 2bd3160
Show file tree

Hide file tree

Showing 2 changed files with 9 additions and 7 deletions.
diff --git a/processing/src/main/java/org/apache/druid/frame/file/FrameFile.java b/processing/src/main/java/org/apache/druid/frame/file/FrameFile.java
@@ -173,7 +173,7 @@ static FrameFile open(
     final EnumSet<Flag> flagSet = flags.length == 0 ? EnumSet.noneOf(Flag.class) : EnumSet.copyOf(Arrays.asList(flags));
 
     if (!file.exists()) {
-      throw new FileNotFoundException(StringUtils.format("File [%s] not found", file));
+      throw new FileNotFoundException(StringUtils.format("File[%s] not found", file));
     }
 
     // Closer for mmap that is shared across all references: either footer only (if file size is larger
@@ -186,7 +186,7 @@ static FrameFile open(
       // Verify minimum file length.
       if (fileLength <
           FrameFileWriter.MAGIC.length + FrameFileWriter.TRAILER_LENGTH + Byte.BYTES /* MARKER_NO_MORE_FRAMES */) {
-        throw new IOE("File [%s] is too short (size = [%,d])", file, fileLength);
+        throw new IOE("File[%s] is too short (size[%,d])", file, fileLength);
       }
 
       // Verify magic.
@@ -195,7 +195,7 @@ static FrameFile open(
       randomAccessFile.readFully(buf, 0, FrameFileWriter.MAGIC.length);
 
       if (!bufMemory.equalTo(0, Memory.wrap(FrameFileWriter.MAGIC), 0, FrameFileWriter.MAGIC.length)) {
-        throw new IOE("File [%s] is not a frame file", file);
+        throw new IOE("File[%s] is not a frame file", file);
       }
 
       // Read number of frames and partitions.
@@ -204,9 +204,9 @@ static FrameFile open(
 
       final int footerLength = bufMemory.getInt(Integer.BYTES * 2L);
       if (footerLength < 0) {
-        throw new ISE("Negative-size footer. Corrupt or truncated file?");
+        throw new ISE("Negative-size footer. Corrupt or truncated file[%s]?", file);
       } else if (footerLength > fileLength) {
-        throw new ISE("Oversize footer. Corrupt or truncated file?");
+        throw new ISE("Oversize footer. Corrupt or truncated file[%s]?", file);
       }
 
       final Memory wholeFileMemory;
@@ -243,7 +243,7 @@ static FrameFile open(
       if (flagSet.contains(Flag.DELETE_ON_CLOSE)) {
         fileCloser.register(() -> {
           if (!file.delete()) {
-            log.warn("Could not delete frame file [%s]", file);
+            log.warn("Could not delete frame file[%s]", file);
           }
           if (byteTracker != null) {
             // Only release the bytes taken by frames, we don't track the header and footer as of now.

diff --git a/processing/src/test/java/org/apache/druid/frame/file/FrameFileWriterTest.java b/processing/src/test/java/org/apache/druid/frame/file/FrameFileWriterTest.java
@@ -75,7 +75,9 @@ public void test_abort_afterAllFrames() throws IOException
 
     MatcherAssert.assertThat(
         e,
-        ThrowableMessageMatcher.hasMessage(CoreMatchers.containsString("Corrupt or truncated file?"))
+        ThrowableMessageMatcher.hasMessage(
+            CoreMatchers.containsString("Negative-size footer. Corrupt or truncated file[")
+        )
     );
   }
 }