sanic
sanic copied to clipboard
sanic-org
Respect error_format defined in app.config when None is given to app.route
If None is given on @app.route, it should respect the global setting instead of go to "auto" check directly.
when the endpoint's error_format is leave blank.
@app.get("/")
def index(req):
raise Exception()
and app.config.FALLBACK_ERROR_FORMAT is set to a specify value
app.config.FALLBACK_ERROR_FORMAT = "json"
As a developer, I would expect such error will format as I defined in the global setting, in this case, it's json, instead of auto.
Currently, it suppress FALLBACK_ERROR_FORMAT setting, and in no case FALLBACK_ERROR_FORMAT will be used.
It force developer define the error_format in every endpoints if auto is not desired.
But indeed its a breaking change.
I agree that auto should only be used in cases in which:
- The global config fallback format is
auto(or not defined) and it has not been overriden for a specific route, or... - The error format has been overriden to
auto.
Every other case should -in my opinion- fallback to the application's config value. While fixing this could be considered a breaking change, I'd personally consider it a bugfix, since the breaking change would've been when this per-route behaviour was introduced and error handling stopped relying on the global config value.
This sounds like another use case for _default as the default value rather than None.
I'd be happy with that (if the default value was that and it meant falling back to the config value) :)
A workaround is possible, catch the general exception and format as desired.
def global_exception_handler(request: Request, exception: Exception) -> HTTPResponse:
"""
Global exception handler
"""
format = request.app.config.FALLBACK_ERROR_FORMAT
if format in RENDERERS_BY_CONFIG:
return RENDERERS_BY_CONFIG[format](request, exception, request.app.debug).render()
else:
# Default render
return HTMLRenderer(request, exception, request.app.debug).render()
and register it to the app
app.error_handler.add(Exception, global_exception_handler)
https://github.com/ling7334/sanic-scaffold/blob/74c63c4604d3f6d1b9d25f6500cf0808ba5d2485/setup.py#L150
@ling7334 are you by any chance calling app.blueprint(...) before updating your config?
@ling7334 are you by any chance calling
app.blueprint(...)before updating your config?
@prryplatypus Sorry for not mention the background.
I'm actually let Sanic load_environment_vars from environment.
I assume it will fetch the env vars once the Sanic is instantiate, before the any router was register.
All good, thanks! Was wondering if it was related at all to another issue someone mentioned in Discord :P
Am I missing something?
app = Sanic(__name__)
app.config.FALLBACK_ERROR_FORMAT = "text"
@app.get("/")
def index(req):
raise Exception()
No matter what value I put in here it comes back as expected. Is there another use case to replicate the bad behavior?
Yeah that's what I remembered from using Sanic too. Felt odd when @ling7334 said otherwise, but maybe we're missing something 🤔