-
-
Notifications
You must be signed in to change notification settings - Fork 723
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
fix: Return proper http response codes for errors #192
fix: Return proper http response codes for errors #192
Conversation
2360079
to
224039d
Compare
@DenverCoder1 let me know if this works |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The error codes also need to be set when throwing the exception
e.g. for errors that should return 404 response, 404 should be passed as the $code
parameter.
throw new InvalidArgumentException("Could not find a user with that name.", 404);
e9c343a
to
5a81b86
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This looks great! This is a very helpful contribution.
There's still a couple suggestions earlier about the spelling of the parameter name.
Also, on line 21 of index.php, maybe a 500 could be sent when the token is missing from the configuration.
if (!isset($_SERVER["TOKEN"])) {
$message = file_exists(dirname(__DIR__ . '../.env', 1))
? "Missing token in config. Check Contributing.md for details."
: ".env was not found. Check Contributing.md for details.";
- renderOutput($message);
+ renderOutput($message, 500);
}
Co-authored-by: Jonah Lawrence <jonah@freshidea.com>
Co-authored-by: Jonah Lawrence <jonah@freshidea.com>
…//github.com/komen205/github-readme-streak-stats into Return-proper-http-response-codes-for-errors
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks great! Thanks for the contribution 🎉
Description
Fixes #171
Type of change
How Has This Been Tested?
composer test
Checklist:
Screenshots