Skip to content

Added more information to the thrown exception for malformed url.#951

Merged
ffang merged 1 commit intoapache:masterfrom
mibesis:feature_config_installer_url
Oct 10, 2019
Merged

Added more information to the thrown exception for malformed url.#951
ffang merged 1 commit intoapache:masterfrom
mibesis:feature_config_installer_url

Conversation

@miroslav-beranic
Copy link
Contributor

Adding more information to the stacktrace what is the problem with the URL and for what Feature it is the problem.

@miroslav-beranic
Copy link
Contributor Author

Copy link
Contributor

@ffang ffang left a comment

Choose a reason for hiding this comment

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

I prefer we still throw MalformedURLException(String detailed message) instead of IOException here.

Could you please revise accordingly?

Thanks!

@miroslav-beranic
Copy link
Contributor Author

Hi,

I was using this now for few days. I would suggest to to have it IOException, with addition of adding origin exception. Why?

Well, as is now ( change in PR ), "origin exception" is hidden, as I did not add the origin exception ( forgot about it ). So, the output does not contain origin error - that the "mvn:" prefix is not recognized.

With suggested solution, there is another problem. I agree exception MalformedURLException would be more suited, but it does not have constructor to accept origin exception, plus in this case it only re-throws exception.

Also, containing method declares as throws IOException, that is also in line with exception throws, so the user of this method can have less catch statements.

IOException is also correct, because of the higher-level exception, that is a result of MalformedURLException -- think of it as: due to malformed URL, file was not found and could not be read ( IOException ).

Based on all this, I suggest and think, that the exception IOException is the correct one to throw, but it is missing additional argument - origin exception, that is MalformedURLException and visible in a stacktrace, also accessible if needed.

Please let me know what you think about this.

Kind Regards,
Miroslav

@ffang
Copy link
Contributor

ffang commented Oct 8, 2019

@miroslav-beranic , your explainition sounds good. Could you please also add origin MalformedURLException when construct the IOException you want to throw?
Thanks!

@ffang ffang merged commit 1d46102 into apache:master Oct 10, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants