ARTEMIS-4926 throw checked URISyntaxException on bad URL #5452
Conversation
jbertram
force-pushed
the
ARTEMIS-4926
branch
from
107e8fe to
722a1ce
Compare
| ActiveMQUtilLogger.LOGGER.unableToParseURLParameterName(parameter.substring(0, p), e); | ||
| continue; |
There was a problem hiding this comment.
Catching and continuing does not actually seem like an improvement to me at all. Personally, I would want it to throw rather than just continue and perhaps do something quite different than I had perhaps intended.
The original complaint seems to be more that this sub-method can throw IAE and the calling methods dont handle that explicitly, or declare so. Feels like they could as easily just declare it throws IAE, or even perhaps wrap with URISyntaxException.
If taking that approach, this could just call parseParameters(URI uri) per the original report and check behviour, rather than exposing this private method.
There was a problem hiding this comment.
That's fair. See my latest push.
There was a problem hiding this comment.
This looks good to me. I had similar thoughts on the original changes as Robbie did, I would generally expect things to fail than to suppress errors and lead people to think configuration was applied when it was not.
jbertram
force-pushed
the
ARTEMIS-4926
branch
from
722a1ce to
69ec3c3
Compare
jbertram
changed the title
No description provided.