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

Return 422 when request is unprocessable by Publishing API #1028

Merged
merged 1 commit into from
Nov 27, 2024

Conversation

brucebolt
Copy link
Member

If the user provides data that is unprocessable by Publishing API (e.g. a base path with the incorrect content ID), we currently return a 500 error and log this to Sentry.

This is not optimal behaviour as there is no application fault here, so no action can be taken by developers after seeing this logged in Sentry.

Instead we should respond with a 422 and return the actual error to the user. Then they can correct the request they are making so it becomes valid.

Trello card

Copy link
Contributor

@callumknights callumknights left a comment

Choose a reason for hiding this comment

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

LGTM 👍 Other than the small comment I left regarding that #TODO

If the user provides data that is unprocessable by Publishing API (e.g.
a base path with the incorrect content ID), we currently return a 500
error and log this to Sentry.

This is not optimal behaviour as there is no application fault here, so
no action can be taken by developers after seeing this logged in Sentry.

Instead we should respond with a 422 and return the actual error to the
user. Then they can correct the request they are making so it becomes
valid.
@brucebolt brucebolt enabled auto-merge November 27, 2024 08:01
@brucebolt brucebolt merged commit 2d9ebe4 into main Nov 27, 2024
8 checks passed
@brucebolt brucebolt deleted the return-422 branch November 27, 2024 08:01
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.

2 participants