Fix BibId error logging and provide API Response payload with proper HTTP status codes. #220
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
…HTTP status codes. If an invalid ID is passed to the system log shows a rather unhelpful null pointer exception. Improve the error logging by throwing an exception with an explicit message. The `ApiResponse` always returns `HTTP 200`, even on errors, not founds, etc... Utilize the exception handlers to return proper HTTP status codes while still preserving the existing behavior of an API Response. Many programs that depend on this might be hard-coded to always expect an HTTP 200, even for errors. Returning a proper HTTP status code might require these programs to be updated. There are some exceptions already being handled that pass along a JSON response and status code already. These are left alone (they already do not return an `ApiResponse`). This only implements the basic functionality of this HTTP status code for specific cases. With this approach, all of the exceptions could be handled this way and the controllers could potentially have reduced need for some conditional checks in them.
rmathew1011
approved these changes
Nov 20, 2025
…nor fixes.
The DEV environment is failing with during an upgrade process:
```json
{"meta":{"status":"SUCCESS","action":null,"message":"Your request was successful","id":null},"payload":{"ArrayList":[]}}
```
This should actually be an error.
Change the code to properly handle this error, thus yielding the following:
```json
{"meta":{"status":"ERROR","action":null,"message":"Failed to get holdings for folio catalog due to: Error 'badArgument': Identifier has invalid structure.","id":null},"payload":{}}
```
This also addresses the situation where multiple errors might be returned.
These errors are concatenated by the exception.
Fix tabbing/spacing in some of the recently changed files.
Bad error messages where HTML is being returned with a mimetype of `application/json` while containing details from the remote server. The HTTP status code being returned is also incorrect. The 404 should not be returned to the user because the current path is valid and does work. If the remote server request fails, then an Internal Server error should instead be presented. Catch `HttpServerErrorException` and `HttpClientErrorException` exceptions when calling `restTemplate.getForObject()`. Provide a new exception, called `RemoteServerError` that provides specific error handling and reporting. The `RemoteServerError` will present a simplified error message to the user, in JSON. A log will be written to that provides additional details, such as the URL. The original response payload will be in the log message, which is likely to be HTML. The following is an example log snippet: ``` 2025-11-21 10:11:22.058 ERROR 25530 --- [nio-9000-exec-1] e.t.c.exception.RestExceptionHandler : Failed to GET https://localhost/bad_url_test/oai?verb=GetRecord&met 404 Not Found: "<html><EOL><EOL><head><title>404 Not Found</title></head><EOL><EOL><body><EOL><EOL><center><h1>404 Not Found</h1></center><EOL><EOL><hr><center>nginx</center><EOL><EOL></bo edu.tamu.catalog.exception.RemoteServerError: Failed to access remote server for folio catalog due to 404 NOT_FOUND. at edu.tamu.catalog.service.FolioCatalogService.restGet(FolioCatalogService.java:1561) at edu.tamu.catalog.service.FolioCatalogService.requestHoldings(FolioCatalogService.java:703) at edu.tamu.catalog.service.FolioCatalogService.getHoldingsByBibId(FolioCatalogService.java:182) at edu.tamu.catalog.controller.CatalogAccessController.getHoldings(CatalogAccessController.java:34) ```
rmathew1011
approved these changes
Nov 21, 2025
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
If an invalid ID is passed to the system log shows a rather unhelpful null pointer exception. Improve the error logging by throwing an exception with an explicit message.
The
ApiResponsealways returnsHTTP 200, even on errors, not founds, etc... Utilize the exception handlers to return proper HTTP status codes while still preserving the existing behavior of an API Response.Many programs that depend on this might be hard-coded to always expect an HTTP 200, even for errors. Returning a proper HTTP status code might require these programs to be updated.
There are some exceptions already being handled that pass along a JSON response and status code already. These are left alone (they already do not return an
ApiResponse).This only implements the basic functionality of this HTTP status code for specific cases. With this approach, all of the exceptions could be handled this way and the controllers could potentially have reduced need for some conditional checks in them.
Note: This is submitted as a draft for pre-reviewing.
Therefore, reviewers are tagged not as a reviewer for merging but instead as a reviewer of the proof of concept designs within this PR.
Additional Changes
Add
HoldingsRequestErrorto handle more problems and perform other minor fixes.The DEV environment is failing with during an upgrade process:
{"meta":{"status":"SUCCESS","action":null,"message":"Your request was successful","id":null},"payload":{"ArrayList":[]}}This should actually be an error.
Change the code to properly handle this error, thus yielding the following:
{"meta":{"status":"ERROR","action":null,"message":"Failed to get holdings for folio catalog due to: Error 'badArgument': Identifier has invalid structure.","id":null},"payload":{}}This also addresses the situation where multiple errors might be returned.
These errors are concatenated by the exception.
Fix tabbing/spacing in some of the recently changed files.
Improve handling of remote server error responses.
Bad error messages where HTML is being returned with a mimetype of
application/jsonwhile containing details from the remote server.The HTTP status code being returned is also incorrect.
The 404 should not be returned to the user because the current path is valid and does work.
If the remote server request fails, then an Internal Server error should instead be presented.
Catch
HttpServerErrorExceptionandHttpClientErrorExceptionexceptions when callingrestTemplate.getForObject().Provide a new exception, called
RemoteServerErrorthat provides specific error handling and reporting.The
RemoteServerErrorwill present a simplified error message to the user, in JSON.A log will be written to that provides additional details, such as the URL.
The original response payload will be in the log message, which is likely to be HTML.
The following is an example log snippet: