[GitHub] [flink] zentol commented on a change in pull request #12845: [FLINK-18519][REST] Send exception to client when app fails to execute

[GitBox <gi...@apache.org>]

zentol commented on a change in pull request #12845:
URL: https://github.com/apache/flink/pull/12845#discussion_r450941335



##########
File path: flink-clients/src/main/java/org/apache/flink/client/deployment/application/DetachedApplicationRunner.java
##########
@@ -78,7 +81,7 @@ public DetachedApplicationRunner(final boolean enforceSingleJobExecution) {
 			ClientUtils.executeProgram(executorServiceLoader, configuration, program, enforceSingleJobExecution, true);
 		} catch (ProgramInvocationException e) {
 			LOG.warn("Could not execute application: ", e);
-			throw new FlinkRuntimeException("Could not execute application.", e);
+			throw new CompletionException(new RestHandlerException("Could not execute application.", HttpResponseStatus.BAD_REQUEST, e));

Review comment:
       It is pretty weird that we throw the RestHandlerException here (along with a CompletionException with no CompletableFuture in sight); it has seemingly little to do with the REST layer.
   I'd suggest to change the JarRunHandler to explicitly handle exceptions thrown by `applicationRunner.run` by changing the `thenApply` into a `handle`.




----------------------------------------------------------------
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.

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