Skip to content

[Issue-2816] - Update exception type in Gson#fromJson to JsonParseException #2829

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

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

Conversation

Shershah03
Copy link

@Shershah03 Shershah03 commented Mar 23, 2025

…eption

Purpose

Closes #2816

Description

As mentioned in #2816 , If com.google.gson.Gson.fromJson(String, Class) throws JsonSyntaxException but in case any one imeplements custom JsonDeserializer then it by default throws JsonParseException.
For any com.google.gson.Gson.fromJson(String, Class) calls , its normal to catch JsonSyntaxException, But in case use of JsonDeserializer, this exception will not be caught.
Ideally fromJson and JsonDeserializer interface should throw same exceptions.
Updating exception type in Gson#fromJson to JsonParseException

Checklist

  • New code follows the Google Java Style Guide
    This is automatically checked by mvn verify, but can also be checked on its own using mvn spotless:check.
    Style violations can be fixed using mvn spotless:apply; this can be done in a separate commit to verify that it did not cause undesired changes.
  • If necessary, new public API validates arguments, for example rejects null
  • New public API has Javadoc
    • Javadoc uses @since $next-version$
      ($next-version$ is a special placeholder which is automatically replaced during release)
  • If necessary, new unit tests have been added
    • Assertions in unit tests use Truth, see existing tests
    • No JUnit 3 features are used (such as extending class TestCase)
    • If this pull request fixes a bug, a new test was added for a situation which failed previously and is now fixed
  • mvn clean verify javadoc:jar passes without errors

@github-advanced-security
Copy link

This pull request sets up GitHub code scanning for this repository. Once the scans have completed and the checks have passed, the analysis results for this pull request branch will appear on this overview. Once you merge this pull request, the 'Security' tab will show more code scanning analysis results (for example, for the default branch). Depending on your configuration and choice of analysis tool, future pull requests will be annotated with code scanning analysis results. For more information about GitHub code scanning, check out the documentation.

@Shershah03
Copy link
Author

@Marcono1234 Modified the Exception as per suggestion

* classOfT
* @see #fromJson(Reader, Class)
* @see #fromJson(String, TypeToken)
*/
public <T> T fromJson(String json, Class<T> classOfT) throws JsonSyntaxException {
public <T> T fromJson(String json, Class<T> classOfT) throws JsonParseException {

Check notice

Code scanning / CodeQL

Confusing overloading of methods Note

Method Gson.fromJson(..) could be confused with overloaded method
fromJson
, since dispatch depends on static types.
@Marcono1234
Copy link
Collaborator

Thanks! I think it would be good to adjust all the other Gson#fromJson methods in the same way, since it affects all of them in the same way.

For some of them it would then list JsonIOException and JsonParseException (throws JsonIOException, JsonParseException). In theory that is redundant because JsonIOException is a subclass. But I think that is not an issue, and only makes it more explicit that users might want to handle JsonIOException separately.


But note that I am not a direct member of this project, so this still needs approval from a project member. So feel free to wait with further changes until further feedback from a project member.

@eamonnmcmanus
Copy link
Member

I agree with @Marcono1234 that we should make this documentation change everywhere it applies. I would go further and say that we should also remove all unchecked exceptions from throws clauses. That's certainly the norm at Google, since declaring unchecked exceptions there has basically zero effect on compilation or execution. Of course we still do want @throws clauses in the javadoc, and if you can improve the accuracy of those then that it is a good service to our users.

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.

Method fromJson throws JsonParseException instead of declared JsonSyntaxException
3 participants