Improve error handling (fixes #897) #1013
Conversation
| res.status(500); | ||
| formatted = err; | ||
| } | ||
| res.send(formatted); |
There was a problem hiding this comment.
This could be improved further by displaying the error message as a full page with normal header and footer (if site was requested successfully). In case of error 401 it can also redirect to /login like some routes already do.
There was a problem hiding this comment.
The first part was mentioned in #982. If you don't do it as part of this PR, I could whip something up. I'm wondering: if the site returns a server error but the site is still fetched, is it a good idea to list the matrix user IDs of instance admins for users to reach out to for support? That information is already on the site response, so no further requests would be required.
| @@ -60,6 +61,8 @@ export class AdminSettings extends Component<any, AdminSettingsState> { | |||
| this.parseMessage = this.parseMessage.bind(this); | |||
| this.subscription = wsSubscribe(this.parseMessage); | |||
|
|
|||
| check_auth(); | |||
There was a problem hiding this comment.
Instead of doing this on the client, I think it would better to handle this on the server and redirect the user before even sending the page that requires auth back.
There was a problem hiding this comment.
To be honest Im not good at frontend dev. If you want you can take over this PR because it takes me forever just to implement the basics. So Id rather stay in Rust :)
A lot of routes are missing basic checks, for example /setup is publicly available even after the site is created. For pages which require auth, the check is handled separately each time when it could be done once at the top level. This is currently work in progress to see if Im on the right track. The same checks should be added to many more routes.