Skip to content

Commit d70ba9f

Browse files
SK-3061: catch generic Exception in all 4 sync bulk methods
bulkInsert/bulkDetokenize/bulkDeleteTokens/bulkTokenize (sync) only caught ApiClientApiException/InterruptedException/ExecutionException, unlike their *Async twins which also catch a generic Exception. This mattered concretely for bulkInsert: its private helper (processBulkInsertSync) calls 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. The other three sync helpers already shield their own interceptor call inside their own try/catch(Exception), so they were not exploitable the same way, but their public methods had the identical outer-catch gap - this is the uniform fix flagged as pending when #412's narrower bulkInsert-only version was reverted (65179ed). Added one regression test per sync method asserting a throwing interceptor surfaces as SkyflowException. Confirmed via TDD: the new bulkInsert test failed before this change (raw IllegalStateException) and passes after; the other three already passed before and continue to pass after. mvn -pl common,flowvault -am test -> 712 (flowvault) + 106 (common/ValidationsTests) tests, 0 failures, BUILD SUCCESS. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
1 parent e22c98a commit d70ba9f

2 files changed

Lines changed: 110 additions & 0 deletions

File tree

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

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -121,6 +121,9 @@ public BulkInsertResponse bulkInsert(BulkInsertRequest insertRequest, BulkInsert
121121
LogUtil.printErrorLog(ErrorLogs.INSERT_RECORDS_REJECTED.getLog());
122122
Throwable cause = e.getCause();
123123
throw new SkyflowException(cause != null && cause.getMessage() != null ? cause.getMessage() : e.getMessage());
124+
} catch (Exception e) {
125+
LogUtil.printErrorLog(ErrorLogs.INSERT_RECORDS_REJECTED.getLog());
126+
throw new SkyflowException(e.getMessage());
124127
}
125128
}
126129

@@ -192,6 +195,9 @@ public BulkDetokenizeResponse bulkDetokenize(BulkDetokenizeRequest detokenizeReq
192195
throw new SkyflowException(e.getMessage());
193196
} catch (ExecutionException e) {
194197
throw new SkyflowException(e.getMessage());
198+
} catch (Exception e) {
199+
LogUtil.printErrorLog(ErrorLogs.DETOKENIZE_REQUEST_REJECTED.getLog());
200+
throw new SkyflowException(e.getMessage());
195201
}
196202
}
197203

@@ -270,6 +276,9 @@ public BulkDeleteTokensResponse bulkDeleteTokens(BulkDeleteTokensRequest deleteT
270276
} catch (ExecutionException | InterruptedException e) {
271277
LogUtil.printErrorLog(ErrorLogs.DELETE_REQUEST_REJECTED.getLog());
272278
throw new SkyflowException(e.getMessage());
279+
} catch (Exception e) {
280+
LogUtil.printErrorLog(ErrorLogs.DELETE_REQUEST_REJECTED.getLog());
281+
throw new SkyflowException(e.getMessage());
273282
}
274283
}
275284

@@ -352,6 +361,9 @@ public BulkTokenizeResponse bulkTokenize(BulkTokenizeRequest tokenizeRequest, Bu
352361
} catch (ExecutionException | InterruptedException e) {
353362
LogUtil.printErrorLog(ErrorLogs.TOKENIZE_REQUEST_REJECTED.getLog());
354363
throw new SkyflowException(e.getMessage());
364+
} catch (Exception e) {
365+
LogUtil.printErrorLog(ErrorLogs.TOKENIZE_REQUEST_REJECTED.getLog());
366+
throw new SkyflowException(e.getMessage());
355367
}
356368
}
357369

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

Lines changed: 98 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1164,4 +1164,102 @@ 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+
}
1193+
1194+
@Test
1195+
public void testBulkDetokenize_throwingInterceptorWrappedAsSkyflowException() throws Exception {
1196+
ApiClient mockApi = Mockito.mock(ApiClient.class);
1197+
RawFlowserviceClient mockRaw = mockRawFlowservice(mockApi);
1198+
stubDetokenizeEcho(mockRaw);
1199+
VaultController controller = createControllerWithMock(mockApi);
1200+
1201+
BulkDetokenizeRequest request = BulkDetokenizeRequest.builder()
1202+
.tokens(Collections.singletonList("token-0"))
1203+
.build();
1204+
1205+
RequestInterceptor interceptor = ctx -> {
1206+
throw new IllegalStateException("sync detokenize interceptor blew up");
1207+
};
1208+
BulkDetokenizeOptions options = BulkDetokenizeOptions.builder().interceptor(interceptor).build();
1209+
1210+
try {
1211+
controller.bulkDetokenize(request, options);
1212+
Assert.fail(EXCEPTION_NOT_THROWN);
1213+
} catch (SkyflowException e) {
1214+
Assert.assertEquals("sync detokenize interceptor blew up", e.getMessage());
1215+
}
1216+
}
1217+
1218+
@Test
1219+
public void testBulkDeleteTokens_throwingInterceptorWrappedAsSkyflowException() throws Exception {
1220+
ApiClient mockApi = Mockito.mock(ApiClient.class);
1221+
RawFlowserviceClient mockRaw = mockRawFlowservice(mockApi);
1222+
stubDeleteTokensEcho(mockRaw);
1223+
VaultController controller = createControllerWithMock(mockApi);
1224+
1225+
BulkDeleteTokensRequest request = BulkDeleteTokensRequest.builder()
1226+
.tokens(Collections.singletonList("token-0"))
1227+
.build();
1228+
1229+
RequestInterceptor interceptor = ctx -> {
1230+
throw new IllegalStateException("sync delete-tokens interceptor blew up");
1231+
};
1232+
BulkDeleteTokensOptions options = BulkDeleteTokensOptions.builder().interceptor(interceptor).build();
1233+
1234+
try {
1235+
controller.bulkDeleteTokens(request, options);
1236+
Assert.fail(EXCEPTION_NOT_THROWN);
1237+
} catch (SkyflowException e) {
1238+
Assert.assertEquals("sync delete-tokens interceptor blew up", e.getMessage());
1239+
}
1240+
}
1241+
1242+
@Test
1243+
public void testBulkTokenize_throwingInterceptorWrappedAsSkyflowException() throws Exception {
1244+
ApiClient mockApi = Mockito.mock(ApiClient.class);
1245+
RawFlowserviceClient mockRaw = mockRawFlowservice(mockApi);
1246+
stubTokenizeEcho(mockRaw);
1247+
VaultController controller = createControllerWithMock(mockApi);
1248+
1249+
ArrayList<BulkTokenizeRequestRecord> records = new ArrayList<>();
1250+
records.add(BulkTokenizeRequestRecord.builder().value("value-0").build());
1251+
BulkTokenizeRequest request = BulkTokenizeRequest.builder().records(records).build();
1252+
1253+
RequestInterceptor interceptor = ctx -> {
1254+
throw new IllegalStateException("sync tokenize interceptor blew up");
1255+
};
1256+
BulkTokenizeOptions options = BulkTokenizeOptions.builder().interceptor(interceptor).build();
1257+
1258+
try {
1259+
controller.bulkTokenize(request, options);
1260+
Assert.fail(EXCEPTION_NOT_THROWN);
1261+
} catch (SkyflowException e) {
1262+
Assert.assertEquals("sync tokenize interceptor blew up", e.getMessage());
1263+
}
1264+
}
11671265
}

0 commit comments

Comments
 (0)