-
Notifications
You must be signed in to change notification settings - Fork 319
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
[Improvement] Possible null pointer exception in CatalogRegister.java #4189
Comments
Hello @justinmclean , can I have a look on this? I think I can resolve this. |
Thanks for taking a look I have assigned it to you |
I am thinking of removing the failedException variable, moving exception handling inside the retry loop, and directly throwing a TrinoException when retries are exhausted. I also thought of adding proper InterruptedException handling. I think these modifications can enhance the method's robustness and error management. What do you think about this? |
I made some changes to fix the NPE and improve the method. Please have a look :- |
@simarjeetss |
This as far as I know is the only case where a NPE could happen in this way |
can u elaborate on this please |
@yuqi1129 are you aware of any other cases where this might occur? |
@justinmclean |
@simarjeetss I think we can initialize failedException to avoid this problem. |
@simarjeetss what do you think? |
@simarjeetss Do you want to still work on this? |
Yes, I'll continue working on this right now. I've been busy with other important work lately. Sorry for the inconvenience |
What would you like to be improved?
In checkCatalogExist, failedException is set to null and then thrown which could cause a NPE if it is not set in the exception block.
How should we improve?
Move
throw failedException;
to the correct place in the catch block?The text was updated successfully, but these errors were encountered: