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

TypeUtils.process(TypeDescriptor, TypeMirror) swallows exception message and stack trace #17974

Closed
wilkinsona opened this issue Aug 27, 2019 · 3 comments

Comments

@wilkinsona
Copy link
Member

commented Aug 27, 2019

Failures in TypeUtils.process(TypeDescriptor, TypeMirror) are very hard to diagnose as the exception message and stack trace is swallowed. See #17650 for an example of a problem that was hard to diagnose due to this.

@Sauhardstark

This comment has been minimized.

Copy link

commented Aug 28, 2019

@wilkinsona, I am a novice so I might be completely wrong but why not add the exception message, name and the stacktrace to the already existing printMessage() method

this.env.getMessager().printMessage(Kind.WARNING, "Failed to generated type descriptor for " + type + " The exception information could go here", this.types.asElement(type));

Thank you

@wilkinsona

This comment has been minimized.

Copy link
Member Author

commented Aug 28, 2019

Thanks for the suggestion, @Sauhardstark.

Including more information in the printed message is certainly one option. The message from the caught exception could probably be included in a way that would work in all environments where the annotation processor is used but I doubt that the stack trace could.

As part of looking at this issue, I'd also like us to take a step back and consider if the exception should be caught at all. Catching an unchecked exception that's thrown by Eclipse's JDT due to (we believe) a bug may not be the right thing to do. It may be better to not catch the exception at all and allow the annotation processing to fail. That should result in the exception's stack trace appearing in the compiler's output in Maven and Gradle and in the error log in Eclipse. This assumes that there aren't other exceptions that may be thrown by the same code that we do want to catch. I don't yet know if that's a reasonable assumption to make.

@snicoll

This comment has been minimized.

Copy link
Member

commented Aug 28, 2019

As part of looking at this issue, I'd also like us to take a step back and consider if the exception should be caught at all.

If memory serves well, the catch was added as we decided to implement a feature in a maintenance release. Playing with generics and different compilers lead to some surprising things for me in the past so I was felling nervous hoping for the best to be honest.

Having said that, I am not against removing the catch in 2.2 and see how it goes if we think this is the right option moving forward.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
3 participants
You can’t perform that action at this time.