From 262347b28f736201a933bf0ce9dddc4d5da01e40 Mon Sep 17 00:00:00 2001 From: Matt Peterson Date: Fri, 23 Aug 2024 07:59:43 -0600 Subject: [PATCH] fix: adjusted exception handling to throw ParseException Signed-off-by: Matt Peterson --- .../block/server/BlockStreamService.java | 3 ++ .../storage/read/BlockAsDirReader.java | 6 ++-- .../persistence/storage/read/BlockReader.java | 6 +++- .../BlockStreamServiceIntegrationTest.java | 4 ++- .../block/server/BlockStreamServiceTest.java | 31 ++++++++++++++++--- .../storage/read/BlockAsDirReaderTest.java | 17 +++++----- .../storage/remove/BlockAsDirRemoverTest.java | 5 +-- .../storage/write/BlockAsDirWriterTest.java | 9 +++--- 8 files changed, 57 insertions(+), 24 deletions(-) diff --git a/server/src/main/java/com/hedera/block/server/BlockStreamService.java b/server/src/main/java/com/hedera/block/server/BlockStreamService.java index c512133df..4a5a2fd07 100644 --- a/server/src/main/java/com/hedera/block/server/BlockStreamService.java +++ b/server/src/main/java/com/hedera/block/server/BlockStreamService.java @@ -209,6 +209,9 @@ private void singleBlock( } catch (IOException e) { LOGGER.log(ERROR, "Error reading block number: {0}", blockNumber); singleBlockResponseStreamObserver.onNext(buildSingleBlockNotAvailableResponse()); + } catch (ParseException e) { + LOGGER.log(ERROR, "Error parsing block number: {0}", blockNumber); + singleBlockResponseStreamObserver.onNext(buildSingleBlockNotAvailableResponse()); } } else { LOGGER.log(ERROR, "Unary singleBlock gRPC method is not currently running"); diff --git a/server/src/main/java/com/hedera/block/server/persistence/storage/read/BlockAsDirReader.java b/server/src/main/java/com/hedera/block/server/persistence/storage/read/BlockAsDirReader.java index a0a6521d7..4ff7484f1 100644 --- a/server/src/main/java/com/hedera/block/server/persistence/storage/read/BlockAsDirReader.java +++ b/server/src/main/java/com/hedera/block/server/persistence/storage/read/BlockAsDirReader.java @@ -84,7 +84,7 @@ class BlockAsDirReader implements BlockReader { */ @NonNull @Override - public Optional read(final long blockNumber) throws IOException { + public Optional read(final long blockNumber) throws IOException, ParseException { // Verify path attributes of the block node root path if (isPathDisqualified(blockNodeRootPath)) { @@ -134,7 +134,7 @@ public Optional read(final long blockNumber) throws IOException { @NonNull private Optional readBlockItem(@NonNull final String blockItemPath) - throws IOException { + throws IOException, ParseException { try (final FileInputStream fis = new FileInputStream(blockItemPath)) { @@ -156,7 +156,7 @@ private Optional readBlockItem(@NonNull final String blockItemPath) throw io; } catch (ParseException e) { LOGGER.log(ERROR, "Error parsing block item: " + blockItemPath, e); - throw new IOException(e); + throw e; } } diff --git a/server/src/main/java/com/hedera/block/server/persistence/storage/read/BlockReader.java b/server/src/main/java/com/hedera/block/server/persistence/storage/read/BlockReader.java index 9f3442b4b..1a24bccdb 100644 --- a/server/src/main/java/com/hedera/block/server/persistence/storage/read/BlockReader.java +++ b/server/src/main/java/com/hedera/block/server/persistence/storage/read/BlockReader.java @@ -16,6 +16,7 @@ package com.hedera.block.server.persistence.storage.read; +import com.hedera.pbj.runtime.ParseException; import edu.umd.cs.findbugs.annotations.NonNull; import java.io.IOException; import java.util.Optional; @@ -33,7 +34,10 @@ public interface BlockReader { * @param blockNumber the block number of the block to read * @return the block with the given block number * @throws IOException if an I/O error occurs fetching the block + * @throws ParseException if the PBJ codec encounters a problem caused by I/O issues, malformed + * input data, or any other reason that prevents the parse() method from completing the + * operation when fetching the block. */ @NonNull - Optional read(final long blockNumber) throws IOException; + Optional read(final long blockNumber) throws IOException, ParseException; } diff --git a/server/src/test/java/com/hedera/block/server/BlockStreamServiceIntegrationTest.java b/server/src/test/java/com/hedera/block/server/BlockStreamServiceIntegrationTest.java index e00a1e896..79019149d 100644 --- a/server/src/test/java/com/hedera/block/server/BlockStreamServiceIntegrationTest.java +++ b/server/src/test/java/com/hedera/block/server/BlockStreamServiceIntegrationTest.java @@ -41,6 +41,7 @@ import com.hedera.hapi.block.*; import com.hedera.hapi.block.stream.Block; import com.hedera.hapi.block.stream.BlockItem; +import com.hedera.pbj.runtime.ParseException; import com.hedera.pbj.runtime.io.buffer.Bytes; import com.lmax.disruptor.BatchEventProcessor; import com.lmax.disruptor.EventHandler; @@ -429,7 +430,8 @@ public void testSubAndUnsubWhileStreaming() throws IOException { } @Test - public void testMediatorExceptionHandlingWhenPersistenceFailure() throws IOException { + public void testMediatorExceptionHandlingWhenPersistenceFailure() + throws IOException, ParseException { final ConcurrentHashMap< EventHandler>, BatchEventProcessor>> diff --git a/server/src/test/java/com/hedera/block/server/BlockStreamServiceTest.java b/server/src/test/java/com/hedera/block/server/BlockStreamServiceTest.java index a6fc43ce0..a031fb1dc 100644 --- a/server/src/test/java/com/hedera/block/server/BlockStreamServiceTest.java +++ b/server/src/test/java/com/hedera/block/server/BlockStreamServiceTest.java @@ -46,6 +46,7 @@ import com.hedera.hapi.block.SubscribeStreamResponse; import com.hedera.hapi.block.stream.Block; import com.hedera.hapi.block.stream.BlockItem; +import com.hedera.pbj.runtime.ParseException; import io.grpc.stub.ServerCalls; import io.grpc.stub.StreamObserver; import io.helidon.webserver.grpc.GrpcService; @@ -127,7 +128,7 @@ public void testProto() throws IOException, NoSuchAlgorithmException { } @Test - void testSingleBlockHappyPath() throws IOException { + void testSingleBlockHappyPath() throws IOException, ParseException { final BlockReader blockReader = BlockAsDirReaderBuilder.newBuilder(config).build(); final BlockStreamService blockStreamService = @@ -168,7 +169,7 @@ void testSingleBlockHappyPath() throws IOException { } @Test - void testSingleBlockNotFoundPath() throws IOException { + void testSingleBlockNotFoundPath() throws IOException, ParseException { // Get the block so we can verify the response payload when(blockReader.read(1)).thenReturn(Optional.empty()); @@ -218,12 +219,11 @@ void testSingleBlockServiceNotAvailable() throws InvalidProtocolBufferException } @Test - public void testSingleBlockIOExceptionPath() throws IOException { + public void testSingleBlockIOExceptionPath() throws IOException, ParseException { final BlockStreamService blockStreamService = new BlockStreamService( streamMediator, blockReader, serviceStatus, blockNodeContext); - // Set the service status to not running when(serviceStatus.isRunning()).thenReturn(true); when(blockReader.read(1)).thenThrow(new IOException("Test exception")); @@ -239,6 +239,27 @@ public void testSingleBlockIOExceptionPath() throws IOException { verify(responseObserver, times(1)).onNext(expectedNotAvailable); } + @Test + public void testSingleBlockParseExceptionPath() throws IOException, ParseException { + final BlockStreamService blockStreamService = + new BlockStreamService( + streamMediator, blockReader, serviceStatus, blockNodeContext); + + when(serviceStatus.isRunning()).thenReturn(true); + when(blockReader.read(1)).thenThrow(new ParseException("Test exception")); + + final com.hedera.hapi.block.protoc.SingleBlockResponse expectedNotAvailable = + buildSingleBlockNotAvailableResponse(); + + // Build a request to invoke the service + final com.hedera.hapi.block.protoc.SingleBlockRequest singleBlockRequest = + com.hedera.hapi.block.protoc.SingleBlockRequest.newBuilder() + .setBlockNumber(1) + .build(); + blockStreamService.protocSingleBlock(singleBlockRequest, responseObserver); + verify(responseObserver, times(1)).onNext(expectedNotAvailable); + } + @Test public void testUpdateInvokesRoutingWithLambdas() { @@ -260,7 +281,7 @@ public void testUpdateInvokesRoutingWithLambdas() { } @Test - public void testProtocParseExceptionHandling() throws IOException { + public void testProtocParseExceptionHandling() { // TODO: We might be able to remove this test once we can remove the Translator class final BlockStreamService blockStreamService = diff --git a/server/src/test/java/com/hedera/block/server/persistence/storage/read/BlockAsDirReaderTest.java b/server/src/test/java/com/hedera/block/server/persistence/storage/read/BlockAsDirReaderTest.java index dbccd9249..e3b7cac84 100644 --- a/server/src/test/java/com/hedera/block/server/persistence/storage/read/BlockAsDirReaderTest.java +++ b/server/src/test/java/com/hedera/block/server/persistence/storage/read/BlockAsDirReaderTest.java @@ -37,6 +37,7 @@ import com.hedera.block.server.util.TestUtils; import com.hedera.hapi.block.stream.Block; import com.hedera.hapi.block.stream.BlockItem; +import com.hedera.pbj.runtime.ParseException; import edu.umd.cs.findbugs.annotations.NonNull; import java.io.FileInputStream; import java.io.FileOutputStream; @@ -82,14 +83,14 @@ public void tearDown() { } @Test - public void testReadBlockDoesNotExist() throws IOException { + public void testReadBlockDoesNotExist() throws IOException, ParseException { final BlockReader blockReader = BlockAsDirReaderBuilder.newBuilder(config).build(); final Optional blockOpt = blockReader.read(10000); assertTrue(blockOpt.isEmpty()); } @Test - public void testReadPermsRepairSucceeded() throws IOException { + public void testReadPermsRepairSucceeded() throws IOException, ParseException { final List blockItems = generateBlockItems(1); final BlockWriter blockWriter = @@ -109,7 +110,7 @@ public void testReadPermsRepairSucceeded() throws IOException { } @Test - public void testRemoveBlockReadPermsRepairFailed() throws IOException { + public void testRemoveBlockReadPermsRepairFailed() throws IOException, ParseException { final List blockItems = generateBlockItems(1); final BlockWriter blockWriter = @@ -148,7 +149,7 @@ public void testRemoveBlockItemReadPerms() throws IOException { } @Test - public void testPathIsNotDirectory() throws IOException { + public void testPathIsNotDirectory() throws IOException, ParseException { final List blockItems = generateBlockItems(1); final Path blockNodeRootPath = Path.of(config.rootPath()); @@ -163,7 +164,7 @@ public void testPathIsNotDirectory() throws IOException { } @Test - public void testRepairReadPermsFails() throws IOException { + public void testRepairReadPermsFails() throws IOException, ParseException { final List blockItems = generateBlockItems(1); @@ -186,7 +187,7 @@ public void testRepairReadPermsFails() throws IOException { } @Test - public void testBlockNodePathReadFails() throws IOException { + public void testBlockNodePathReadFails() throws IOException, ParseException { // Remove read perm on the root path removePathReadPerms(Path.of(config.rootPath())); @@ -202,7 +203,7 @@ public void testBlockNodePathReadFails() throws IOException { } @Test - public void testParseExceptionHandling() throws IOException { + public void testParseExceptionHandling() throws IOException, ParseException { final List blockItems = generateBlockItems(1); final BlockWriter blockWriter = @@ -236,7 +237,7 @@ public void testParseExceptionHandling() throws IOException { // Read the block. The block item file is corrupted, so the read should fail with a // ParseException - assertThrows(IOException.class, () -> blockReader.read(1)); + assertThrows(ParseException.class, () -> blockReader.read(1)); } public static void removeBlockReadPerms(int blockNumber, final PersistenceStorageConfig config) diff --git a/server/src/test/java/com/hedera/block/server/persistence/storage/remove/BlockAsDirRemoverTest.java b/server/src/test/java/com/hedera/block/server/persistence/storage/remove/BlockAsDirRemoverTest.java index dca6661b8..bbe824d93 100644 --- a/server/src/test/java/com/hedera/block/server/persistence/storage/remove/BlockAsDirRemoverTest.java +++ b/server/src/test/java/com/hedera/block/server/persistence/storage/remove/BlockAsDirRemoverTest.java @@ -32,6 +32,7 @@ import com.hedera.block.server.util.TestUtils; import com.hedera.hapi.block.stream.Block; import com.hedera.hapi.block.stream.BlockItem; +import com.hedera.pbj.runtime.ParseException; import java.io.IOException; import java.nio.file.Files; import java.nio.file.Path; @@ -63,7 +64,7 @@ public void setUp() throws IOException { } @Test - public void testRemoveNonExistentBlock() throws IOException { + public void testRemoveNonExistentBlock() throws IOException, ParseException { // Write a block final var blockItems = PersistTestUtils.generateBlockItems(1); @@ -96,7 +97,7 @@ public void testRemoveNonExistentBlock() throws IOException { } @Test - public void testRemoveBlockWithPermException() throws IOException { + public void testRemoveBlockWithPermException() throws IOException, ParseException { // Write a block final List blockItems = PersistTestUtils.generateBlockItems(1); diff --git a/server/src/test/java/com/hedera/block/server/persistence/storage/write/BlockAsDirWriterTest.java b/server/src/test/java/com/hedera/block/server/persistence/storage/write/BlockAsDirWriterTest.java index 871d59a28..d11965037 100644 --- a/server/src/test/java/com/hedera/block/server/persistence/storage/write/BlockAsDirWriterTest.java +++ b/server/src/test/java/com/hedera/block/server/persistence/storage/write/BlockAsDirWriterTest.java @@ -35,6 +35,7 @@ import com.hedera.block.server.util.TestUtils; import com.hedera.hapi.block.stream.Block; import com.hedera.hapi.block.stream.BlockItem; +import com.hedera.pbj.runtime.ParseException; import edu.umd.cs.findbugs.annotations.NonNull; import java.io.IOException; import java.nio.file.Files; @@ -79,7 +80,7 @@ public void tearDown() { } @Test - public void testWriterAndReaderHappyPath() throws IOException { + public void testWriterAndReaderHappyPath() throws IOException, ParseException { // Write a block final List blockItems = PersistTestUtils.generateBlockItems(1); @@ -116,7 +117,7 @@ public void testWriterAndReaderHappyPath() throws IOException { } @Test - public void testRemoveBlockWritePerms() throws IOException { + public void testRemoveBlockWritePerms() throws IOException, ParseException { final List blockItems = PersistTestUtils.generateBlockItems(1); @@ -177,7 +178,7 @@ public void testUnrecoverableIOExceptionOnWrite() throws IOException { } @Test - public void testRemoveRootDirReadPerm() throws IOException { + public void testRemoveRootDirReadPerm() throws IOException, ParseException { final List blockItems = PersistTestUtils.generateBlockItems(1); final BlockWriter blockWriter = @@ -205,7 +206,7 @@ public void testRemoveRootDirReadPerm() throws IOException { } @Test - public void testPartialBlockRemoval() throws IOException { + public void testPartialBlockRemoval() throws IOException, ParseException { final List blockItems = PersistTestUtils.generateBlockItems(3); final BlockRemover blockRemover = new BlockAsDirRemover(Path.of(testConfig.rootPath()), FileUtils.defaultPerms);