Skip to content

[FLINK-4824] [client] CliFrontend shows misleading error message#2662

Closed
greghogan wants to merge 3 commits into
apache:masterfrom
greghogan:4824_clifrontend_shows_misleading_error_message_when_main_method_returns_before_env_execute
Closed

[FLINK-4824] [client] CliFrontend shows misleading error message#2662
greghogan wants to merge 3 commits into
apache:masterfrom
greghogan:4824_clifrontend_shows_misleading_error_message_when_main_method_returns_before_env_execute

Conversation

@greghogan
Copy link
Copy Markdown
Contributor

When a Flink program throws ProgramParametrizationException the optional message is printed to stderr and the stacktrace is ignored.

I modified Gelly's JaccardIndex driver to use this functionality as an example.

When a Flink program throws ProgramParametrizationException the
optional message is printed to stderr and the stacktrace is ignored.
@mxm
Copy link
Copy Markdown
Contributor

mxm commented Oct 20, 2016

Thanks for the PR @greghogan! Having a custom exception for missing arguments to a user program is a good approach. However, it requires the author of the program to use the custom exception. At least, we would have to adapt all the included examples. Additionally, it would be nice to throw another custom exception when no Flink job was generated during execution of the jar (which might be because of missing arguments). Currently, we simply throw a ProgramInvocationException which could look like a serious error to the user when merely arguments are missing.

So +1 but we might do some follow-ups to fully solve the issue.

case a message is printed to stderr without a stacktrace.
@greghogan
Copy link
Copy Markdown
Contributor Author

@mxm thanks for the review. I added a second commit which I think satisfies your request. When no job is executed then the message is printed to stderr without a stacktrace.

Copy link
Copy Markdown
Contributor

@mxm mxm left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the update. Looks great @greghogan! We could consider removing the the zero-args constructor from ProgramParametrizationException.

+1 to merge otherwise

* Creates a <tt>ProgramParametrizationException</tt>.
*/
public ProgramParametrizationException() {
super();
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think we should not even allow to skip the message here. This will simplify the code further and display some message for the user.

private int handleParametrizationException(ProgramParametrizationException e) {
String message = e.getMessage();
if (message != null) {
System.err.println(message);
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This block could be removed if we got rid of the zero-args constructor in ProgramParametrizationException.

@asfgit asfgit closed this in c4783c8 Oct 24, 2016
liuyuzhong pushed a commit to liuyuzhong/flink that referenced this pull request Dec 5, 2016
When a command-line program is run but no Flink job is executed the
message to the user is now displayed without the stacktrace.

When a Flink program throws ProgramParametrizationException the
optional message is printed to stderr without a stacktrace.

This closes apache#2662
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants