making default reponse text, with optional formatting to be passed #1905
Conversation
There was a problem hiding this comment.
First, thank you so much for adding this. I have been myself meaning to make this change for some time now.
With that said, I do not want to just remove the HTML page as it does have value. Instead, I think it should be configurable with three options:
- HTML page (as it was)
- Plain text
- JSON document
For now, I do not want to change the existing behavior. So, I would suggest leaving the HTML as default, with an opt in for the other styles. After the next LTS, we can debate which one should be default.
|
A custom error handling middleware is probably the best way to go for machine-readable error messages. Different APIs require vastly different error messages ranging from empty documents (only status code) to plain text message and JSON in various formats. I think that changing this properly needs more work than simply this patch, which would still show internal server errors as HTML if I am not mistaken (tracebacks etc. are HTML formatted). |
|
@ahopkins thank you for your comments. I agree with the proposal of having a configurable option. It should be something which is configurable by a user though right? I couldn't see a clean way in the current design to pass user preferences til that point (shouldn't be a global config for the reasons @Tronic mentioned..of different APIs consuming different types of errors). @Tronic I could extend it to the other responses, but I think a configurable option throughout would make sense. To handle the multiple responses, should a parameter be added to |
|
Error handling middleware has the advantage that it can also contain the logic for determining which type of response is required. Some apps may determine this by request content-type, others might use path such as |
|
Clearly exception handlers and middleware are the recommended methodology in a production server. However, when prototyping something or getting something started, it is not convenient and often annoying that the exceptions come back in HTML format. I am not suggesting that we offer full blown solutions since there will clearly need to be considerations that the individual developer needs to make. But, having text and JSON options will be considerably more user-friendly than the current state. Furthermore, I disagree and think it absolutely should be globally defined. We are talking about the default fallback handler. if request.app.config.DEFAULT_ERROR_FORMAT == "text":
resp = text(...)
elif request.app.config.DEFAULT_ERROR_FORMAT == "json":
resp = json(...)
else:
resp = html(...) |
|
Yes, agree with @ahopkins that's the |
I am generally not a fan of this. Or at least trying to support this from within Sanic. The problem is that running services like Sanic generally will be scaled horizontally. Syncing config values is tricky and undoubtedly would need another service to achieve that. HOWEVER, I am working on a new user guide with tutorials. A good place to put something like this would be there as a "how could you achieve this" type of reference material. |
|
My opinion on this, with highest preference first: |
|
Closing in favor of #1937 |
The default error handling should not return html text. Sanic itself can be run as a headless server just for API consumption (ex. CLIs). It would make sense for errors to be propagated with the text they are originally supplied with rather than with surround html tags. While custom error handling is of course possible, I do believe that this should be the default behavior.
Additionally, custom formatting can be passed as a string as before, just pass
{{body}}:f"<!DOCTYPE html><meta charset=UTF-8><title>{title}</title><style>{TRACEBACK_STYLE}</style>\n <h1>⚠️ {title}</h1><p>{{body}}\n {_render_traceback_html(request, exception)}"