Abacn commented on code in PR #30336:
URL: https://github.com/apache/beam/pull/30336#discussion_r1492957558


##########
sdks/java/core/src/main/java/org/apache/beam/sdk/io/FileBasedSource.java:
##########
@@ -472,23 +473,28 @@ public synchronized FileBasedSource<T> getCurrentSource() 
{
     @Override
     protected final boolean startImpl() throws IOException {
       FileBasedSource<T> source = getCurrentSource();
-      this.channel = 
FileSystems.open(source.getSingleFileMetadata().resourceId());
-      if (channel instanceof SeekableByteChannel) {
-        SeekableByteChannel seekChannel = (SeekableByteChannel) channel;
-        seekChannel.position(source.getStartOffset());
-      } else {
-        // Channel is not seekable. Must not be a subrange.
-        checkArgument(
-            source.mode != Mode.SINGLE_FILE_OR_SUBRANGE,
-            "Subrange-based sources must only be defined for file types that 
support seekable "
-                + " read channels");
-        checkArgument(
-            source.getStartOffset() == 0,
-            "Start offset %s is not zero but channel for reading the file is 
not seekable.",
-            source.getStartOffset());
-      }
+      ResourceId resourceId = source.getSingleFileMetadata().resourceId();
+      try {
+        this.channel = FileSystems.open(resourceId);
+        if (channel instanceof SeekableByteChannel) {
+          SeekableByteChannel seekChannel = (SeekableByteChannel) channel;
+          seekChannel.position(source.getStartOffset());
+        } else {
+          // Channel is not seekable. Must not be a subrange.
+          checkArgument(
+              source.mode != Mode.SINGLE_FILE_OR_SUBRANGE,
+              "Subrange-based sources must only be defined for file types that 
support seekable "
+                  + " read channels");
+          checkArgument(
+              source.getStartOffset() == 0,
+              "Start offset %s is not zero but channel for reading the file is 
not seekable.",
+              source.getStartOffset());
+        }
 
-      startReading(channel);
+        startReading(channel);
+      } catch (IOException e) {
+        LOG.error(String.format("Failed to process %s.", resourceId), e);

Review Comment:
   We should re-throw the error, e.g.
   ```
   throw new RuntimeException(String.format("Failed to process %s.", 
resourceId), e);
   ```
   and narrow down the try block to that "Failed to process" actually happen, 
that is startReading call? 
   
   
   



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: github-unsubscr...@beam.apache.org

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org

Reply via email to