Nicer error handling #1024
Nicer error handling #1024
Conversation
|
Whoa nice! These look really good. I should be able to test later today. |
| import { ErrorPageData, setIsoData } from "../../utils"; | ||
| import { ErrorPage } from "../app/error-page"; | ||
|
|
||
| class ErrorGuard extends Component<any, any> { |
There was a problem hiding this comment.
I just thought of something. I could probably handle this on the server. It would probably both reduce bundle size and make my hacky use of 'routeData' unnecessary.
There was a problem hiding this comment.
I just know that the server will return the json {"error: "string"}, for any failed request. Not sure how that translates for this.
In jerboa at least, all these errors do proper exception throws, so I can catch them, read the error message, and do actions based on that.
There was a problem hiding this comment.
My comment was unclear. I meant on the server side of the UI app that serves the pages, but not the actual Lemmy server.
As to doing different actions based on different error messages, that's definitely doable. We just need to define what should be done for those error messages. At the very least there should be a way to handle displaying that a user is banned (I even think there's an open issue for that).
|
@dessalines before you merge, I'd like to give what I mentioned here a try. |
|
I've tried making the previously mentioned change. It wasn't quite what I was going for, but I think its an improvement over how I originally handled |
|
@dessalines I almost forgot: I'm going to have to add translations for those error messages. |
|
That looks great! But please dont link to the issue tracker, it will lead to a lot of issues opened for tech support. Instead it can link to !lemmy_support and the Matrix chat. That way users can help each other and devs dont have to help with every little problem. |
| @@ -61,6 +61,7 @@ | |||
| "isomorphic-cookie": "^1.2.4", | |||
| "jwt-decode": "^3.1.2", | |||
| "lemmy-js-client": "0.17.2-rc.17", | |||
| "lodash": "^4.17.21", | |||
There was a problem hiding this comment.
lodash seems unused, and I'd def like to avoid it if possible.
There was a problem hiding this comment.
lodash is only being used in webpack.config.js. I originally tried having it as a dev dependency, but this caused issues when I tested the prod Dockerfile. This shouldn't be included in the bundle.
There was a problem hiding this comment.
yeah i got this issue too when trying to run docker compose against lemmy-ui
| import { ErrorPageData, setIsoData } from "../../utils"; | ||
| import { ErrorPage } from "../app/error-page"; | ||
|
|
||
| class ErrorGuard extends Component<any, any> { |
There was a problem hiding this comment.
I just know that the server will return the json {"error: "string"}, for any failed request. Not sure how that translates for this.
In jerboa at least, all these errors do proper exception throws, so I can catch them, read the error message, and do actions based on that.
| // in production will get a more generic message. | ||
| if (process.env.NODE_ENV === "development") { | ||
| errorPageData.error = error; | ||
| } |
There was a problem hiding this comment.
Im not sure this makes sense. The reason not to show error messages is usually for security, but anyway the error messages are available if you use the API directly.
Another reason might be to keep things simple for non-techie users. But even if they dont understand the error, there can be a text telling them to copy-paste the error message to admins/devs. That can be very helpful for debugging.
|
@Nutomic @dessalines I made a blurb for the error code. Here is what it looks like: If you like the verbiage, I'll submit a translation. On the topic of errors, I have an idea for Lemmy errors that I'm looking for feedback on. |
|
Looks good to me. |
This reverts commit 7fbb12a.
…nal network" This reverts commit 498de66.
…e1917/lemmy-ui into nicer-error-hnadling
I took up Nutomic's offer and implemented what was being implemented in #1013.
This is what the 404 error page looks like:
This is for a 500 error:
Besides making the error pages, I made it so that making sure an authenticated route has expected behavior with logouts and redirects only requires a change to one place. This should hopefully help make things easier to maintain.