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

Link to troubleshooting guide from exception messages #2357

Conversation

Marcono1234
Copy link
Collaborator

@Marcono1234 Marcono1234 commented Mar 25, 2023

⚠️ Maybe wait with merging this until #2323 is merged.

Purpose

Adds troubleshooting guide links to some exception messages; improves unit tests

Description

As shortly discussed in #1219 (comment) (TypeToken validation is not implemented here though) adding troubleshooting guide links to exception messages hopefully makes it easier for users to understand what the exception means and how to solve the issue.

This approach again uses custom HTML anchors in a Markdown document even though we had bad experience with this before (#2286), but it appears using only lowercase IDs is supported by GitHub (though possibly not officially, see open question https://github.com/orgs/community/discussions/50962). The custom HTML anchors allow us to produce short stable links.

Open questions:

  • Possibly we could also consider including the troubleshooting section ID (e.g. "T1") in the troubleshooting guide headers, but that might become confusing if we decide to reorder, remove or merge sections in the future and the section IDs are then not in order anymore. What is your opinion?
  • Currently for all MalformedJsonExceptions it refers to the troubleshooting guide; maybe we should reduce this only to specific variants?

Checklist

  • New code follows the Google Java Style Guide
  • 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

@Marcono1234
Copy link
Collaborator Author

Marcono1234 commented Mar 26, 2023

Or maybe instead of the anchors t1, t2, ... it would be better to use more expressive ones, such as unexpected-structure, class-unsupported, .... What do you think?

Edit: Have changed this now; feedback regarding the anchor names is appreciated!

@eamonnmcmanus
Copy link
Member

This looks great! I'm sure it will be very helpful for users.
I'm holding off on merging as requested.

@Marcono1234
Copy link
Collaborator Author

I'm holding off on merging as requested.

Or could you please merge it now already? My intention was to avoid conflicts for #2323, however it looks like that has already conflicts with master now, and also it targets the strictness branch, so it might take some time until it is finally merged to master.

@eamonnmcmanus eamonnmcmanus merged commit cbad1aa into google:master Apr 15, 2023
@Marcono1234 Marcono1234 deleted the marcono1234/exception-troubleshooting-links branch April 15, 2023 23:06
eamonnmcmanus pushed a commit that referenced this pull request May 31, 2023
* Link to troubleshooting guide from exception messages

* Add examples to troubleshooting guide

* Use proper anchor names for troubleshooting guide
tibor-universe pushed a commit to getuniverse/gson that referenced this pull request Sep 14, 2024
* Link to troubleshooting guide from exception messages

* Add examples to troubleshooting guide

* Use proper anchor names for troubleshooting guide
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