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

docs: Add possible exceptions to API documentation #1569

Merged
merged 1 commit into from
May 27, 2024

Conversation

MoritzWeber0
Copy link
Member

@MoritzWeber0 MoritzWeber0 commented May 15, 2024

The Collaboration Manager backend can throw a bunch of different errors. Those errors were only documented in the code by scanning the complete call trace, which is unintuitive for users who just want to talk to the API.

Therefore, I've migrated all legacy occurrences of fastapi.HTTPException to our own exception format. Those exceptions can be registered as response for a route using the responses attribute. A helper function api_exceptions can be used to automatically translate the exceptions to the OpenAPI format. When registered, the exceptions are automatically added to the API documentation.

This PR mainly focuses on the initial implementation of the behaviour and doesn't register exceptions for all routes. However, I did it for some routes, e.g. the "Request session" route, which can be used as example.

In the API documentation, exceptions are grouped by status codes. The generated responses are visible in the generated documentation clients. The large amount of error responses might be too bloated for some generated clients, e.g. our frontend client. Therefore, the addition of error responses in the OpenAPI schema can be disabled by setting the environment variable CAPELLACOLLAB_SKIP_OPENAPI_ERROR_RESPONSES=1.

In SwaggerUI, the individual schemas slightly hidden. They are not displayed in example section, instead, you have to navigate to "Schema" to see all available error responses.

image

In ReDoc, they are better visible:

image

The best option is the right side in the ReDoc documentation where you can select the error in a dropdown to see the the returned JSON:
image

Resolves #883.

Copy link

github-actions bot commented May 15, 2024

A Storybook preview is available for commit 02d1ae5.
View Storybook
View Chromatic build

Copy link

codecov bot commented May 15, 2024

Codecov Report

Attention: Patch coverage is 72.92225% with 101 lines in your changes missing coverage. Please review.

Project coverage is 79.73%. Comparing base (52cfc68) to head (45512d5).
Report is 146 commits behind head on main.

Files Patch % Lines
...nd/capellacollab/core/authentication/jwt_bearer.py 25.00% 9 Missing ⚠️
...ellacollab/settings/modelsources/t4c/exceptions.py 75.00% 6 Missing ⚠️
...pellacollab/settings/modelsources/t4c/interface.py 37.50% 5 Missing ⚠️
backend/capellacollab/core/responses.py 87.09% 2 Missing and 2 partials ⚠️
backend/capellacollab/projects/routes.py 33.33% 3 Missing and 1 partial ⚠️
...d/capellacollab/core/authentication/injectables.py 75.00% 3 Missing ⚠️
...lab/projects/toolmodels/modelsources/t4c/routes.py 40.00% 3 Missing ⚠️
backend/capellacollab/sessions/routes.py 66.66% 3 Missing ⚠️
...ttings/modelsources/t4c/repositories/exceptions.py 72.72% 3 Missing ⚠️
...b/settings/modelsources/t4c/repositories/routes.py 25.00% 3 Missing ⚠️
... and 38 more
Additional details and impacted files
@@            Coverage Diff             @@
##             main    #1569      +/-   ##
==========================================
+ Coverage   79.52%   79.73%   +0.20%     
==========================================
  Files         172      185      +13     
  Lines        5778     5971     +193     
  Branches      666      666              
==========================================
+ Hits         4595     4761     +166     
- Misses       1034     1064      +30     
+ Partials      149      146       -3     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@MoritzWeber0 MoritzWeber0 force-pushed the add-exceptions-to-openapi branch from 2675424 to 74e6084 Compare May 15, 2024 12:36
@MoritzWeber0 MoritzWeber0 changed the base branch from main to feat-add-mccabe-complexity-check May 15, 2024 14:10
@MoritzWeber0 MoritzWeber0 force-pushed the add-exceptions-to-openapi branch 3 times, most recently from 387c2de to 8f3de15 Compare May 21, 2024 14:26
@MoritzWeber0 MoritzWeber0 force-pushed the feat-add-mccabe-complexity-check branch from 7a4d9e8 to 9468ce9 Compare May 21, 2024 14:27
@MoritzWeber0 MoritzWeber0 force-pushed the add-exceptions-to-openapi branch 3 times, most recently from 22b2b50 to 7725c5d Compare May 22, 2024 11:42
Copy link
Contributor

@dominik003 dominik003 left a comment

Choose a reason for hiding this comment

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

LGTM! There are just a few minor comments that should be addressed before we merge this.

backend/capellacollab/core/authentication/jwt_bearer.py Outdated Show resolved Hide resolved
backend/capellacollab/core/authentication/jwt_bearer.py Outdated Show resolved Hide resolved
backend/capellacollab/core/responses.py Show resolved Hide resolved
backend/capellacollab/projects/exceptions.py Outdated Show resolved Hide resolved
@MoritzWeber0 MoritzWeber0 force-pushed the add-exceptions-to-openapi branch from 7725c5d to 3f9b718 Compare May 27, 2024 12:27
Base automatically changed from feat-add-mccabe-complexity-check to main May 27, 2024 13:03
Exceptions are grouped by status codes. The exceptions have to be
registered on the route using the responses attribute.
A helper function `api_exceptions` can be used to automatically
translate the exceptions to the OpenAPI format.
@MoritzWeber0 MoritzWeber0 force-pushed the add-exceptions-to-openapi branch from 3f9b718 to 45512d5 Compare May 27, 2024 13:06
@MoritzWeber0 MoritzWeber0 enabled auto-merge May 27, 2024 13:07
Copy link

sonarcloud bot commented May 27, 2024

Quality Gate Passed Quality Gate passed

Issues
1 New issue
0 Accepted issues

Measures
0 Security Hotspots
No data about Coverage
0.0% Duplication on New Code

See analysis details on SonarCloud

@MoritzWeber0 MoritzWeber0 merged commit 156102c into main May 27, 2024
29 of 30 checks passed
@MoritzWeber0 MoritzWeber0 deleted the add-exceptions-to-openapi branch May 27, 2024 13:11
MoritzWeber0 added a commit that referenced this pull request Jun 11, 2024
When the refresh token was expired, the backend sent the error
"TOKEN_SIGNATURE_EXPIRED". The frontend did then request a new refresh token,
leading to an infinite loop and a DDOS attack on our backend.

The error was accidentially added in #1569.
MoritzWeber0 added a commit that referenced this pull request Jun 11, 2024
When the refresh token was expired, the backend sent the error
"TOKEN_SIGNATURE_EXPIRED". The frontend did then request a new refresh token,
leading to an infinite loop and a DDOS attack on our backend.

The error was accidentially added in #1569.
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.

Consider custom exceptions in OpenAPI documentation
2 participants