Show custom alert for 504 error from nginx #1052
Conversation
| @@ -441,8 +441,7 @@ | |||
| "mixin": { | |||
| "request": { | |||
| "alert": { | |||
| "fileSize": "The file “{name}” that you are trying to upload is larger than the 100 MB limit.", | |||
| "entityTooLarge": "The data that you are trying to upload is too large.", | |||
| "fileSize": "The file “{name}” that you are trying to upload is larger than the 100 MB limit." | |||
There was a problem hiding this comment.
This message is not used in src/util/request.js, so I'm leaving it where it is for now. I think it's very likely that we'll move it at some point in the future (probably as part of #675).
| "error": { | ||
| // @transifexKey mixin.request.alert.entityTooLarge | ||
| "413": "The data that you are trying to upload is too large.", | ||
| "504": "Your request took too long, and Central stopped waiting for it." |
There was a problem hiding this comment.
@lognaturel @alyblenkin, let me know if you can think of a better message to show to users.
There was a problem hiding this comment.
(Just for the 504, we've already shipped the message for the 413.)
There was a problem hiding this comment.
Why did it take too long? Can they do anything to prevent it from timing out?
I don't think we need to say and "Central stopped waiting" because it seems like we gave up :)
We could say "Your request took too long because X. Try again."
There was a problem hiding this comment.
Really good to get your feedback, @alyblenkin! I do think "why did you give up?!" is a likely reaction to that message. I think most users of websites have never thought about the idea that there has to be some processing time cutoff or the service could get stuck forever.
Who is the improved message aimed at? What action do we want them to take?
One idea is to aim it at end users and to get them to contact us. "The latest action you attempted timed out. Please contact [email protected] with a description of what you were doing."
Another is to aim it at server admins and give them the idea that they could adjust the timeout. "The latest action exceeded the configured nginx timeout."
Maybe a hybrid? "The latest action exceeded the configured nginx timeout. Please contact [email protected] with a description of what you were doing."
That gives admins a clue of an action they could take today. It also encourages frustrated users to give us more info on when this happens.
There was a problem hiding this comment.
I like "The latest action you attempted timed out. Please contact [email protected] with a description of what you were doing" because it’s actionable and user-friendly! From a non-technical perspective, "configured nginx" might require some Googling, so perhaps sticking with the first option is better unless you think server admins would immediately understand what's happening?
There was a problem hiding this comment.
That sounds good to me as well!
This is a small thing, but would the message read alright without the word "latest"? On some pages, it's possible for the user to take multiple actions at once. I don't think this is the most likely thing, but it's within the realm of possibility that an earlier action was the one to result in the error, while the latest action was successful.
There was a problem hiding this comment.
Ah that's an interesting point. So maybe something like:
Your action could not be completed because it timed out. If this issue continues happening, please contact [email protected] with a description of what you were doing.
We know that it's possible for users to receive a 504 response from nginx if the request takes too long (more than 2 minutes). We've seen this most recently at getodk/central#171 (comment), but it's also come up in the past. Right now, a user who receives a 504 will just see, "Something went wrong: error code 504." The goal of this PR is to show a nicer error message instead. The user may still need to contact us for assistance, but at least they won't need to look up what a 504 is.
What has been done to verify that this works as intended?
New unit test.
Why is this the best possible solution? Were any other approaches considered?
I proposed the new error text as part of the discussion at getodk/central#171.
Before submitting this PR, please make sure you have:
npm run testandnpm run lintand confirmed all checks still pass OR confirm CircleCI build passes