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

dev/core#4298 - respond with 400 not 500 errors if wrong params passed to CiviMail urls #26236

Merged

Conversation

ufundo
Copy link
Contributor

@ufundo ufundo commented May 16, 2023

@civibot
Copy link

civibot bot commented May 16, 2023

No issue was found matching the number given in the pull request title. Please check the issue number.

@civibot
Copy link

civibot bot commented May 16, 2023

(Standard links)

@civibot civibot bot added the master label May 16, 2023
@ufundo ufundo force-pushed the 400-not-500-errors-for-bad-requests branch from 57ae254 to d2de7c1 Compare May 16, 2023 18:32
@ufundo ufundo force-pushed the 400-not-500-errors-for-bad-requests branch from d2de7c1 to 4a740a2 Compare May 17, 2023 21:19
@ufundo
Copy link
Contributor Author

ufundo commented May 17, 2023

just updated for the style complaints

@totten
Copy link
Member

totten commented Jul 20, 2023

I'm not certain about the use-case in which there's a functional distinction, but (going by the specs generally) -- these conditions do look more like a 400 than a 500 👍 .

@larssandergreen
Copy link
Contributor

I think this makes sense and it would be a nice improvement to remove these unhelpful errors from logs.

@demeritcowboy demeritcowboy merged commit 4d90166 into civicrm:master Jul 27, 2023
@demeritcowboy
Copy link
Contributor

I'm not certain about the use-case in which there's a functional distinction

One nice thing is with this change it no longer appears in the logs, flooding them with junk. As an admin 500 errors are something you might want to look into, but 400 is almost always bots.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants