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

Exceptions are swallowed when using withCompilation flag #974

Closed
idanakav opened this issue May 3, 2022 · 6 comments · Fixed by #1096
Closed

Exceptions are swallowed when using withCompilation flag #974

idanakav opened this issue May 3, 2022 · 6 comments · Fixed by #1096
Assignees
Labels
bug Something isn't working
Milestone

Comments

@idanakav
Copy link

idanakav commented May 3, 2022

Hey, per title, when returnOkOnError=false && withCompilation=true I would expect to fail with calling
AnalysisResult.compilationError instead of proceeding with the compilation.

Any reason we would not want to fail fast in that scenario?

Happy to submit a PR in case you agree with this case.

@neetopia
Copy link
Contributor

neetopia commented May 3, 2022

They are not really swallowed, it is intentional. The reason is that we want to provide most details about errors in the code, KSP is not able to provide any diagnostics due to its limitation, therefore when withCompilation flag is enabled, we want the users still have a chance to get the diagnostics from the compiler for troubleshooting.

@Gzernov
Copy link

Gzernov commented May 5, 2022

I agree with @idanakav. When I have withCompilation enabled, I would still like my build to fail if there is an error.
@neetopia 's argument also sounds reasonable. While it may not be a good solution in general, one more flag can be added that forces build to fail on error (even if that means that user is no longer have access to the proper diagnostics)
Thoughts?

@neetopia
Copy link
Contributor

neetopia commented May 5, 2022

Fair enough, let me consider it. Will keep you updated.

@neetopia neetopia added the enhancement New feature or request label May 5, 2022
@idanakav
Copy link
Author

idanakav commented Aug 9, 2022

@neetopia wondering if you had a chance to give it any thoughts? do you think an extra flag would be the way to go? I'm happy to help with contributing.

@neetopia neetopia added this to the 1.0.7 milestone Aug 9, 2022
@neetopia
Copy link
Contributor

neetopia commented Aug 9, 2022

Thanks for the remind, added to next milestone for discussion.

@neetopia
Copy link
Contributor

This actually looks like a bug to me after taking a deep look, basically the logic here ignores returnOkOnError when withCompilation is set to true. Current plan is to fix it rather than introducing a new flag.

@neetopia neetopia added bug Something isn't working and removed enhancement New feature or request labels Aug 10, 2022
@neetopia neetopia self-assigned this Aug 16, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants