You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
We recently faced one issue where below method returned false Optional.empty() for a file which did exist. On checking the code found issue with below code in RepositoryFileApi class.
Issue:
Method GitLabApi.createOptionalFromException(var5) above doesn't do status code checks on GitLabApiException before returning Optional.empty(). This method is used across all getOptionalXXX() methods in all Apis and this means all getOptionalXXX() methods in all repositories has this issue.
Expected Behaviour:
This method should first check on 'httpStatus' == 404 to ensure that file indeed doesn't exist on server and then return optional.empty(). All other exceptions should be rethrown wrapped as Suitable Exception.
The text was updated successfully, but these errors were encountered:
The problem I see with "rethrow": all the getOptionalXXX() methods are not throwing any Exception. So we should maybe wrap it into a Runtime exception. Not sure if it makes sense or not.
I looked at the code and I am also not convinced by the mechanism of storing the exception in a map.
Yep, I found it later all methods returning Optional have this issue, all use same method GitlabApi.createOptionalFromException() which never throws exception no matter what the status code is.
amitt00r
changed the title
RepositoryFileApi's getOptionalFileInfo method can return false Optional.empty()
getOptionalXXX() method can return false Optional.empty()
Mar 11, 2024
We recently faced one issue where below method returned false Optional.empty() for a file which did exist. On checking the code found issue with below code in RepositoryFileApi class.
Issue:
Method GitLabApi.createOptionalFromException(var5) above doesn't do status code checks on GitLabApiException before returning Optional.empty(). This method is used across all getOptionalXXX() methods in all Apis and this means all getOptionalXXX() methods in all repositories has this issue.
Expected Behaviour:
This method should first check on 'httpStatus' == 404 to ensure that file indeed doesn't exist on server and then return optional.empty(). All other exceptions should be rethrown wrapped as Suitable Exception.
The text was updated successfully, but these errors were encountered: