Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Give more helpful error message when an invalid alias is encountered #205

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

willmurnane
Copy link

The current error message when an invalid alias is passed in is misleading. I revised it to make it more user-friendly.

@@ -117,7 +117,7 @@ public Object decode(String alias, U value) {
if (encoder != null)
return encoder.decode(value);

throw new TypeDecodingException("An unknown alias [" + value + "] was encountered");
throw new TypeDecodingException("An unknown alias [" + alias + "] was encountered while decoding value [" + value + "]. Valid aliases are " + String.join(", ", aliasMapping.keySet()));
Copy link
Member

@eawagner eawagner Aug 19, 2016

Choose a reason for hiding this comment

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

Two problems with this.

  1. We shouldn't assume that the value is easily representable as a string. There are instances for example where byte[] are used instead of strings. These will print out as a cryptic error message. That being said the error message was incorrectly using the value instead of the alias.
  2. String.join is a java 8 method. Mango is compatible to Java 6 to work with older applications in production.

Copy link
Author

Choose a reason for hiding this comment

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

  1. Trying toString sounds better than nothing. Maybe it'll confuse the issue by printing garbage, but maybe it'll be helpful for figuring out what data was being processed when this problem was encountered.
  2. Mango uses the diamond operator all over the place, and that requires 1.7. I'll rewrite to use guava's Joiner instead.

Copy link
Member

Choose a reason for hiding this comment

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

Yes, but printing the value doesn't add anything to the value of the error message when the alias is the reason for the exception so lets not do that. If the exception was from the act of decoding then yes it would potentionally make sense to try to show something about the value.

Oops, I meant Java 7. Guava Joiner is fine.

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