Skip to content
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

[API] Improve status reporting content/headers #425

Merged
merged 10 commits into from
Jul 24, 2024

Conversation

rinkp
Copy link
Member

@rinkp rinkp commented Jul 22, 2024

Description

In this PR, we update the way normal and abnormal situations are reported on in the API. Authentication and authorization is handled separately from error reporting. We also distinguish between API routes and regular interactive routes for displaying models.

This also introduces a test endpoint for 404 and 500 errors.

Related issues/external references

Resolves GH-423, will now return a 500 instead of a 302 on internal server errors.

Types of changes

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to change)
    Client applications that rely on a 302 status code to detect failures may experience issues, API version was upgraded to major version 3 as a result.
  • Documentation improvement (no changes to code)
  • Other (please specify)

@rinkp rinkp requested a review from tomudding July 22, 2024 23:15
tomudding
tomudding previously approved these changes Jul 24, 2024
Copy link
Member

@tomudding tomudding left a comment

Choose a reason for hiding this comment

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

Thank you for this amazing work @rinkp! I have some small comments/nitpicks but they are non-blocking.

module/User/src/Listener/AuthorizationListener.php Outdated Show resolved Hide resolved
module/Database/src/Controller/ApiController.php Outdated Show resolved Hide resolved
module/User/src/Listener/AuthorizationListener.php Outdated Show resolved Hide resolved
module/User/src/Listener/AuthorizationListener.php Outdated Show resolved Hide resolved
@rinkp rinkp force-pushed the feat/apiinternalerroris500 branch from ab50ba1 to c08633e Compare July 24, 2024 20:58
@tomudding tomudding self-requested a review July 24, 2024 21:00
Copy link
Member

@tomudding tomudding left a comment

Choose a reason for hiding this comment

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

❤️ (merge at your own convenience)

@rinkp rinkp merged commit b3d6ec9 into GEWIS:main Jul 24, 2024
4 checks passed
@rinkp rinkp deleted the feat/apiinternalerroris500 branch July 24, 2024 21:22
github-actions bot added a commit that referenced this pull request Jul 24, 2024
rinkp: Merge pull request GH-425 from rinkp/feat/apiinternalerroris500

Reviewers: Tom Udding <[email protected]>

Co-authored-by: rinkp <[email protected]>
rinkp added a commit to GEWIS/gewisdb-ts-client that referenced this pull request Jul 24, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

API endpoints should throw a 500 on error, instead of a 302
2 participants