search

maces / fastapi-htmx

Extension for FastAPI to make HTMX easier to use.
GNU Lesser General Public License v3.0
229 stars 9 forks source link

MissingFullPageTemplateError generates 500 error to clients #9

Closed wfaler closed 8 months ago

wfaler commented 9 months ago

I really like the start of this. I'm quite new to Fast API and htmx. One thing I noticed though, with the example, calling /customers from a client without the wrapping template generates MissingFullPageTemplateError (probably fine) and an http 500 error (less optimal).

Is there any way to change this into a 400 (Bad Request), as it is arguably the client causing the error by calling an endpoint wrong? Throwing a 500 would in most cases cause havoc in an observability system, given the amount of spurious errors that are in fact not errors.

If someone can point me in the right direction on how to fix this, happy to try to fix it myself and make a contribution.

maces commented 9 months ago

Hi there,

thanks :)

Good point you are making there. I linked a PR with a change to return a 400er instead. I would like to iterate over one aspect before merging though:

TL;DR: Who to "blame" (server / client) and how to help them to avoid getting "blame" (DX)

The switch from 400 to 500 implies, that the developers did not wrongly configure the frontend. E.g. for example if a route can be called via a deep link directly, but has no fullpage template provided and the user reloads the page. In that case the error is provoked but the user did things to be expected by a user. With a 400er this case would not be as visible. The server still does not support it though. But is it the server or the clients fault?

AFAIK detecting WHY the wrong request was made is probably rather out of scope for this library i guess (analytics, tracking?), but choosing which case is assumed by default can be simply switched like in the PR.

Since the user experience would be broken either way (400 or 500) a better error message, especially for the less tech savvy should be added anyway.

With these considerations in mind I wonder if this can be simply solved by providing additional docs and examples on how to test for the above example case? Effectively moving the "blame" to the developers but also helping for it to not be a problem?

What do you think?

maces commented 8 months ago

The 400er response is now merged and will be released with the next release

maces commented 8 months ago

Is now released and published.