Skip to content

Replace json_error_response() with abort()#1502

Merged
williamjallen merged 1 commit into
Kitware:masterfrom
williamjallen:json_error_response-cleanup
Jun 21, 2023
Merged

Replace json_error_response() with abort()#1502
williamjallen merged 1 commit into
Kitware:masterfrom
williamjallen:json_error_response-cleanup

Conversation

@williamjallen
Copy link
Copy Markdown
Collaborator

#1479 introduced a centralized error-handling system which returns a JSON error message for API routes and an HTML error page for pages with content which is meant to be consumed by humans. The recommended way to trigger the new error handling system is through Laravel's abort() function which throws and then catches an exception, rather than using the current json_error_response() function which simply returns a JSON response and then exits. Exiting is problematic for our simpletest-based tests, because they won't catch the fact that the program exited prematurely with an error without running the remainder of the test suite.

This PR replaces most of the existing usages of json_error_response() with an equivalent abort(). The remaining usages of json_error_response() are more difficult to resolve and will be fixed in future PRs once I refactor the underlying infrastructure.

@williamjallen williamjallen force-pushed the json_error_response-cleanup branch from 7f46f2c to 8a279aa Compare June 16, 2023 13:48
Copy link
Copy Markdown
Member

@josephsnyder josephsnyder left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

builddetails test still fails to report correctly but I know there is more work to be done. All other aspects look in good shape!

@williamjallen williamjallen merged commit 0ab96f2 into Kitware:master Jun 21, 2023
@williamjallen williamjallen deleted the json_error_response-cleanup branch June 21, 2023 19:12
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants