XComp commented on code in PR #23428:
URL: https://github.com/apache/flink/pull/23428#discussion_r1345443798


##########
flink-runtime-web/src/main/java/org/apache/flink/runtime/webmonitor/handlers/utils/JarHandlerUtils.java:
##########
@@ -189,7 +189,11 @@ public PackagedProgram toPackagedProgram(Configuration 
configuration) {
                         .setArguments(programArgs.toArray(new String[0]))
                         .build();
             } catch (final ProgramInvocationException e) {
-                throw new CompletionException(e);
+                throw new CompletionException(

Review Comment:
   I'm not sure whether we assume here that the `ProgramInvocationException` is 
caused by something the user provided (which would allow the BAD_REQUEST 
response). 
[PackagedProgram#extractContainedLibraries](https://github.com/apache/flink/blob/c08bef1f7913eb1c416c278354fd62b82b172549/flink-clients/src/main/java/org/apache/flink/client/program/PackagedProgram.java#L554)
 throws a `ProgramInvocationException` for unknown IO errors, for instance, 
which would be actually an internal issue, wouldn't it?



##########
flink-runtime/src/main/java/org/apache/flink/runtime/rest/handler/AbstractHandler.java:
##########
@@ -242,8 +242,14 @@ private CompletableFuture<Void> handleException(
             return CompletableFuture.completedFuture(null);
         }
         int maxLength = flinkHttpObjectAggregator.maxContentLength() - 
OTHER_RESP_PAYLOAD_OVERHEAD;
-        if (throwable instanceof RestHandlerException) {
-            RestHandlerException rhe = (RestHandlerException) throwable;
+        if (throwable instanceof RestHandlerException
+                || throwable.getCause() instanceof RestHandlerException) {

Review Comment:
   That's a strange change (checking for both, the exception and the 
exception's cause), don't you think? It might be an indication that we should 
do the change somewhere else. WDYT? :thinking: 



-- 
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: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]

Reply via email to