Wrap all javaparser getType calls in Try #4666
Merged
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
nikoqwietai
approved these changes
Jun 13, 2024
itsacoderepo
approved these changes
Jun 13, 2024
ml86
approved these changes
Jun 14, 2024
johannescoetzee
force-pushed
the
johannes/javasrc-safe-get-type
branch
from
90dc3ee
to
d8ab25a
Compare
ml86
requested changes
Jun 14, 2024
| case failure: Failure[_] => | ||
| val exceptionType = failure.exception.getClass | ||
|
|
||
| val shouldLog = loggedExceptionCounts.get(exceptionType) match { |
There was a problem hiding this comment.
You will need to use an atomic read + update operation on the concurrent map. Like this the code is not thread safe.
ml86
approved these changes
Jun 14, 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.
Related to #4665
The AstCreator is crashing on a customer project due to an
UnsolvedSymbolException. As mentioned in the PR, there isn't enough information in the logs to locate the exact issue, but my theory is that it is due to agetTypecall somewhere. For this PR, I've wrapped allgetTypecalls in aTry(unless they were already wrapped) since the JavaParser symbol solver and the interactions between the various kinds of solvers are too complex to easily tell when these calls should be safe and when not.Revision 2: I've changed
TrytotryWithSafeStackOverflow, which also logs stack traces for errors now. This isn't called during regular type resolution, so we won't log every failed type (since those are often to be expected), but we should keep an eye on javasrc log sizes and more selectively log these if it turns out to be too much