Issue #11270 - Improve XmlConfiguration reporting of Resource location during error #11345
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
…n during error
…lconfig-error-paths
sbordet
requested changes
Feb 13, 2024
| finally | ||
| { | ||
| if (parser instanceof Closeable) | ||
| ((Closeable)parser).close(); | ||
| if (parser instanceof Closeable closable) |
| */ | ||
| public XmlConfiguration(Resource resource) throws SAXException, IOException | ||
| public XmlConfiguration(Resource resource) |
There was a problem hiding this comment.
Uhm, this is now a breaking change, as user code that did try/catch (IOException) will now get a compilation error.
There was a problem hiding this comment.
Guess I can leave those throws declarations to not change the signature,
Doesn't matter if that they will never happen with the other changes in this PR.
| @@ -280,10 +277,14 @@ public XmlConfiguration(Resource resource, Map<String, Object> idMap, Map<String | |||
| _idMap = idMap == null ? new HashMap<>() : idMap; | |||
| _propertyMap = properties == null ? new HashMap<>() : properties; | |||
| } | |||
| catch (Throwable t) | |||
| { | |||
| throw new XmlConfigurationException("Bad Xml Config in " + this, t); | |||
There was a problem hiding this comment.
Suggested change
| throw new XmlConfigurationException("Bad Xml Config in " + this, t); | |
| throw new XmlConfigurationException("Bad Jetty XML configuration in " + this, t); |
| @@ -465,14 +466,14 @@ public Object configure() throws Exception | |||
| } | |||
| catch (NoSuchMethodException x) | |||
| { | |||
| throw new IllegalStateException(String.format("No matching constructor %s in %s", oClass, _configuration)); | |||
| throw new XmlConfigurationException("No matching constructor " + oClass); | |||
There was a problem hiding this comment.
I would throw IllegalStateException here as before, because A) in many other places you did not convert to throw XmlConfigurationException (you converted just a couple), and it's caught and rethrown as XmlConfigurationException anyway.
| @@ -429,7 +430,7 @@ public Object configure(Object obj) throws Exception | |||
| if (oClass != null && !oClass.isInstance(obj)) | |||
| { | |||
| String loaders = (oClass.getClassLoader() == obj.getClass().getClassLoader()) ? "" : "Object Class and type Class are from different loaders."; | |||
| throw new IllegalArgumentException("Object of class '" + obj.getClass().getCanonicalName() + "' is not of type '" + oClass.getCanonicalName() + "'. " + loaders + " in " + _configuration); | |||
| throw new XmlConfigurationException("Object of class '" + obj.getClass().getCanonicalName() + "' is not of type '" + oClass.getCanonicalName() + "'. " + loaders); | |||
…lconfig-error-paths
Cleaned up code in toString(). Signed-off-by: Simone Bordet <simone.bordet@gmail.com>
sbordet
approved these changes
Feb 27, 2024
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Attempt to unify all of the Exception flows on
XmlConfigurationto always report the location of theXmlConfigurationresource (if able to).Fixes: #11270