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

Remove "impossible" IOException in readTree() and readValue() ObjectMapper methods which accept Strings #1675

Closed
matthew-pwnieexpress opened this issue Jun 22, 2017 · 6 comments
Milestone

Comments

@matthew-pwnieexpress
Copy link

Unless I'm missing something, methods which read from Strings should never be able to throw an IOException. It seems like it would be a nice API improvement if such ObjectMapper no longer threw the (checked) IOException, which must be explicitly handled and ignored. I'm not 100% sure, but this might also apply to the byte[] methods as well.

Relevant methods:

  • readTree(String)
  • readValue(String, _)
@cowtowncoder
Copy link
Member

@matthew-pwnieexpress True, although the problem is that methods it has to call on JsonParser after construction will still declare IOException so trying to remove this exception adds code. Further, JsonProcessingException, a sub-type of IOException, will need to be thrown.

So I don't see much value in adding extra code to slightly reduce declaration of kinds of exceptions that may be thrown.

@matthew-pwnieexpress
Copy link
Author

matthew-pwnieexpress commented Jun 22, 2017

@cowtowncoder Sure. Though there is already the workaround in place for writing the value as String/byte[], so there is prior art/existing API using this pattern within the class.

@cowtowncoder
Copy link
Member

True. I'll have a look when I get chance.

@johnlinp
Copy link

+1 for this.

I feel kind of awkward in the following scenario:

try {
    JsonNode jsonNode = mapper.readTree(jsonString);
}
catch (JsonProcessingException e) {
    // sure, I can handle this.
}
catch (IOException e) {
    // what should I do?
}

Or this:

try {
    JsonNode jsonNode = mapper.readTree(jsonString);
}
catch (IOException e) {
    // actually handling JsonProcessingException
}

Both code snippets are weird for me.

@cowtowncoder
Copy link
Member

Quick note: Jackson 3.x will change JsonProcessingException into unchecked one, see #2177, and wrap IOException, so this particular problem will be solved.

But we can definitely consider changes to byte[] and String for 2.10 now. I will add this to one of "simple" issues on WIP list at https://github.com/FasterXML/jackson-future-ideas/wiki/Jackson-Work-in-Progress

@johnlinp
Copy link

@cowtowncoder Thanks for the explanations. Very helpful.

@cowtowncoder cowtowncoder changed the title Impossible IOException in readTree and readValue ObjectMapper methods which accept Strings? Remove "impossible" IOException in readTree() and readValue() ObjectMapper methods which accept Strings Feb 4, 2019
@cowtowncoder cowtowncoder added this to the 2.10.0 milestone Feb 4, 2019
rjlohan pushed a commit to aws-cloudformation/cloudformation-cli-java-plugin that referenced this issue Nov 20, 2019
rjlohan pushed a commit to aws-cloudformation/cloudformation-cli-java-plugin that referenced this issue Nov 20, 2019
* Bump jackson.version from 2.9.7 to 2.10.1

Bumps `jackson.version` from 2.9.7 to 2.10.1.

Updates `jackson-databind` from 2.9.7 to 2.10.1
- [Release notes](https://github.com/FasterXML/jackson/releases)
- [Commits](https://github.com/FasterXML/jackson/commits)

Updates `jackson-dataformat-cbor` from 2.9.7 to 2.10.1
- [Release notes](https://github.com/FasterXML/jackson-dataformats-binary/releases)
- [Commits](FasterXML/jackson-dataformats-binary@jackson-dataformats-binary-2.9.7...jackson-dataformats-binary-2.10.1)

Updates `jackson-datatype-jsr310` from 2.9.7 to 2.10.1

Updates `jackson-modules-java8` from 2.9.7 to 2.10.1
- [Release notes](https://github.com/FasterXML/jackson-modules-java8/releases)
- [Commits](FasterXML/jackson-modules-java8@jackson-modules-java8-2.9.7...jackson-modules-java8-2.10.1)

Signed-off-by: dependabot[bot] <[email protected]>

* Fixes related to jackson-databind upgrade

* Minor change to exception handling to account for behavioural change in FasterXML/jackson-databind#1675
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

No branches or pull requests

3 participants