feat: Added enumeration of HTTP response status codes

[mpa12]

Summary

Added enumeration of HTTP response status codes

Resolves #59

Type of change

• New feature (added a non-breaking change which adds functionality)

How Has This Been Tested?

• Ran tests with composer test

[DenverCoder1]

This is fine so far, though the main thing to resolve for this issue is to make our server return different response codes for invalid url combinations.

Currently every combination of parameters gives a 200 response, but there are some possible error cases where it is not "ok" and should set the http response code to something else.

For example:

• Missing or invalid parameters could be a 422 (unprocessable entity)

  • missing lines parameter - https://readme-typing-svg.demolab.com?lines=

  • negative height, width, pause, duration, font size, font weight, etc - https://readme-typing-svg.demolab.com?width=-1&lines=X

• If there is an issue getting a font from Google fonts, assuming an error message appears to the user, it could be 503 (service unavailable)

[mpa12]

@DenverCoder1 As I understand it, you need query validation. In addition to statuses, maybe it can also return the error text? For example, if some parameter is missing, then write Missing parameter <parameter_name>

[DenverCoder1]

It already displays error messages, it just doesn't have different status codes for different errors, for example, this page shows the response code is 200, when it would be more accurate to say 422 since there was an error

One possible way to do this could be to pass the codes to the exceptions, and call http_response_code on the error's code before rendering an error svg.

This is a PR to a similar issue in a different repository for reference: https://github.com/DenverCoder1/github-readme-streak-stats/pull/192/files

[mpa12]

I didn't display an error message when I failed to get the font. This could cause animations to crash for some users. In the current version, if you can't get the desired font, the default font is used

[mpa12]

[DenverCoder1]

Thanks so much for all the work you've put in to solving this!

While this does work, as far as the implementation, there are a few things I'd do differently for code readability and maintainability.

Constructors should not typically have side effects, in other words, using the construction of the exception to change the response code is unexpected and as a principle should be avoided.

I think the most expected place to find this would be in the RenderedController class when setting the headers.

• A $statusCode field could be added to RendererController defaulting to 200, but set in the constructor's catch block if an exception raised.
• The status code field should be set based on the exception for extensibility. For example, using the exception code. If it's not set (i.e. it's 0), the status code could be set to 500 as that would indicate an unknown server error occurred. Alternatively, the class of the exception could be checked.
• In the setHeaders() method of RendererController which is called from index.php, the http_status_code() function can be used to set the status code using the value in the status code field.

Feel free to let me know your thoughts and if you have any questions.

Thanks

[mpa12]

Quite a good point! I'll fix it

[DenverCoder1]

Thanks for the continued work, looks great so far

[mpa12]

Thank you for your attention to my work and valuable comments! I took all your recommendations into account and corrected the errors

[DenverCoder1]

Looks perfect! Thanks for the contribution 🎉