Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Plans that fail to load give no hints to what went wrong #250

Open
micheal-w-wells opened this issue Oct 11, 2019 · 3 comments
Open

Plans that fail to load give no hints to what went wrong #250

micheal-w-wells opened this issue Oct 11, 2019 · 3 comments
Labels
Enhancement New feature or request

Comments

@micheal-w-wells
Copy link
Collaborator

"Retry or go back" doesn't give us much to go on. Could be an API issue (not callable, db constraint, etc) could be problem with React not liking the data coming from API, who knows.

We should pass on a more descriptive error to the user (and include something friendly like who to reach out to) if there is a problem. This will speed up troubleshooting issues that will come up.

@micheal-w-wells micheal-w-wells added the Enhancement New feature or request label Oct 11, 2019
@calebissharp
Copy link
Contributor

@micheal-w-wells I'm not sure if it's a good idea to show stack traces in production. I did implement a change at some point that logs errors into the console when not in production. Really, we shouldn't have to rely on users to provide error logs to us, but instead use some kind of logging service. I don't know if the government has any preferences on what to use for that.

@micheal-w-wells
Copy link
Collaborator Author

While I agree no one needs to look at a big stack dump like jenkins gave us the other afternoon, in past experience there is operational benefit to error messages being a bit more informative. I'd say it should at least break things down to "error rendering page" (which wouldn't be logged on a server anyway) or "error communicating with API" as the user friendly messages, and underneath perhaps the more techy error (but not the whole stack dump). This gets even more valuable with increasing number of glued together systems. We'll have to tackle logging too at some point, you aren't wrong there.

@calebissharp
Copy link
Contributor

Gotcha, I see what you mean. In this case, "error fetching plan" should be the only error that shows this page. If you look at the try-catch block, all that is deals with is 1) fetching the plan and 2) casting the plan against the schema. In a production environment, the schema cast should never fail once we iron out all the bugs in it. However, a fetch to the backend could definitely fail if the user loses internet connectivity or the API goes offline for some reason. For that reason, I don't think there's much more specificity we could supply in this specific error page. I'm ok with logging the actual thrown error in the console in production though (the same as what's configured for dev now).

There are other areas of the app where errors get thrown, which currently results in that "blank screen" issue that @LisaMoore1 has had a few times. With error boundaries, we could handle those kinds of errors differently.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

2 participants