-
Notifications
You must be signed in to change notification settings - Fork 0
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
TSPS-404 Throw correct exception for Sam user not found #223
base: develop
Are you sure you want to change the base?
Conversation
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This seems correct with respect to Sam's behavior, but I wonder why 403/forbidden is being returned instead of 404/not found in the first place? I would expect a 404 if a user isn't found.
} catch (final ApiException e) { | ||
if (e.getCode() == HttpStatus.NOT_FOUND.value()) { | ||
if (e.getCode() == HttpStatus.NOT_FOUND.value() | ||
|| e.getCode() == HttpStatus.FORBIDDEN.value()) { | ||
throw new UnauthorizedException("User not found", e); | ||
} else { | ||
throw new InternalServerErrorException(e); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It seems odd to have an exception that contains a http status and then use it to map to a different http status. Another approach would be to map the ApiException's status directly to the new exception value
throw new ErrorReportException(e, HttpStatus.valueOf(e.getCode())) {};
Teaspoons uses TCL's SamUserFactory to check that the user is registered in Sam.
However, if a user is not registered in Sam/Terra, TCL is currently throwing an InternalServerErrorException that results in a 500 response, with the error message stating that the user was not found in Sam and an error code of 403 (FORBIDDEN). That's because TCL expects Sam to return a 404 (NOT_FOUND), so it doesn't properly handle the 403 that Sam returns in this case .
This PR updates the logic so that TCL treats both a 404 and a 403 as "User not found".
Teaspoons Jira ticket: https://broadworkbench.atlassian.net/browse/TSPS-404