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

Improve error reporting in cherrypy #63836

Merged
merged 4 commits into from
Mar 10, 2023

Conversation

whytewolf
Copy link
Collaborator

What does this PR do?

fixes 400 error reporting in cherrypy to give users more context about issue.

What issues does this PR fix or reference?

Fixes: #63835

Previous Behavior

400 error was generic

New Behavior

adds exception message from exception that caused 400 to 400 error and log error.

Merge requirements satisfied?

[NOTICE] Bug fixes or features added to Salt require tests.

Commits signed with GPG?

No

@whytewolf whytewolf requested a review from a team as a code owner March 7, 2023 19:27
@whytewolf whytewolf requested review from twangboy and removed request for a team March 7, 2023 19:27
s0undt3ch
s0undt3ch previously approved these changes Mar 7, 2023
@anilsil anilsil changed the title better error reporting in cherrypy Improve error reporting in cherrypy Mar 7, 2023
@s0undt3ch
Copy link
Collaborator

@whytewolf Mind adding a test case verifying that we now see the message that we previously didn't?

@s0undt3ch s0undt3ch added the Needs-Testcase PR needs test cases written, or the issue is about a bug/feature that needs test cases label Mar 9, 2023
@s0undt3ch s0undt3ch added this to the Sulfur v3006.0 milestone Mar 9, 2023
@whytewolf
Copy link
Collaborator Author

@whytewolf Mind adding a test case verifying that we now see the message that we previously didn't?

no problem. test added. took a little longer than i wanted to do though.

Copy link
Collaborator

@s0undt3ch s0undt3ch 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.

Ch3LL
Ch3LL previously approved these changes Mar 9, 2023
@Ch3LL Ch3LL added the Sulfur v3006.0 release code name and version label Mar 9, 2023
twangboy
twangboy previously approved these changes Mar 9, 2023
@s0undt3ch s0undt3ch removed the Needs-Testcase PR needs test cases written, or the issue is about a bug/feature that needs test cases label Mar 10, 2023
s0undt3ch
s0undt3ch previously approved these changes Mar 10, 2023
@s0undt3ch s0undt3ch dismissed stale reviews from twangboy, Ch3LL, and themself via 91a69d3 March 10, 2023 07:30
@s0undt3ch
Copy link
Collaborator

Approved and tests are passing. Merging.

@s0undt3ch s0undt3ch merged commit f06a02e into saltstack:3006.x Mar 10, 2023
garethgreenaway added a commit that referenced this pull request Mar 10, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Sulfur v3006.0 release code name and version
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants