Skip to content

Commit 7133bb2

Browse files
SK-3061: fix bulkInsert(sync) interceptor-exception leak
processBulkInsertSync called insertBatchFutures - which synchronously invokes the caller's RequestInterceptor - before entering its own try block, so a throwing interceptor escaped bulkInsert() as a raw, undeclared exception instead of the documented SkyflowException. processBulkDetokenizeSync/processBulkDeleteTokensSync/processBulkTokenizeSync already call their own *BatchFutures inside their own try/catch(Exception), which is why they were not exploitable the same way. This moves insertBatchFutures's call inside processBulkInsertSync's existing try, matching that same structure, instead of adding a new catch-all to the 4 public sync methods (the broader fix reverted here). Added a regression test asserting a throwing interceptor surfaces as SkyflowException from bulkInsert. Confirmed via TDD: failed before this change (raw IllegalStateException), passes after. mvn -pl common,flowvault -am test -> 709 (flowvault) + 106 (common/ValidationsTests) tests, 0 failures, BUILD SUCCESS. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
1 parent 59f21f9 commit 7133bb2

2 files changed

Lines changed: 27 additions & 1 deletion

File tree

flowvault/src/main/java/com/skyflow/vault/controller/VaultController.java

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -778,9 +778,9 @@ private BulkInsertResponse processBulkInsertSync(
778778
) throws ExecutionException, InterruptedException, SkyflowException {
779779
LogUtil.printInfoLog(InfoLogs.PROCESSING_BATCHES.getLog());
780780
List<BulkInsertResponseRecord> records = new ArrayList<>();
781-
List<CompletableFuture<BulkInsertResponse>> futures = this.insertBatchFutures(insertRequest, interceptor, cfg);
782781

783782
try {
783+
List<CompletableFuture<BulkInsertResponse>> futures = this.insertBatchFutures(insertRequest, interceptor, cfg);
784784
CompletableFuture<Void> allFutures = CompletableFuture.allOf(futures.toArray(new CompletableFuture[0]));
785785
try {
786786
allFutures.join();

flowvault/src/test/java/com/skyflow/vault/controller/VaultControllerTests.java

Lines changed: 26 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1164,4 +1164,30 @@ public void testBulkTokenize_interceptorInvokedOncePerBatchWithDistinctContext()
11641164
Mockito.verify(mockRaw, Mockito.times(EXPECTED_BATCH_COUNT)).tokenize(any(), captor.capture());
11651165
assertInterceptorRanOncePerBatch(interceptor, captor.getAllValues());
11661166
}
1167+
1168+
@Test
1169+
public void testBulkInsert_throwingInterceptorWrappedAsSkyflowException() throws Exception {
1170+
ApiClient mockApi = Mockito.mock(ApiClient.class);
1171+
RawFlowserviceClient mockRaw = mockRawFlowservice(mockApi);
1172+
stubInsertEcho(mockRaw);
1173+
VaultController controller = createControllerWithMock(mockApi);
1174+
1175+
Map<String, Object> data = new HashMap<>();
1176+
data.put("name", "john");
1177+
ArrayList<InsertRequestRecord> records = new ArrayList<>();
1178+
records.add(BulkInsertRequestRecord.builder().tableName("table1").data(data).build());
1179+
BulkInsertRequest request = BulkInsertRequest.builder().records(records).build();
1180+
1181+
RequestInterceptor interceptor = ctx -> {
1182+
throw new IllegalStateException("sync insert interceptor blew up");
1183+
};
1184+
BulkInsertOptions options = BulkInsertOptions.builder().interceptor(interceptor).build();
1185+
1186+
try {
1187+
controller.bulkInsert(request, options);
1188+
Assert.fail(EXCEPTION_NOT_THROWN);
1189+
} catch (SkyflowException e) {
1190+
Assert.assertEquals("sync insert interceptor blew up", e.getMessage());
1191+
}
1192+
}
11671193
}

0 commit comments

Comments
 (0)