Collect: Add error handling to Match Exactly sync

[seadowg]

Acceptance

• [x] Given I've set up Match Exactly and have navigated to Fill Blank Forms
  And my server is down/not available (UnknownHostException)
  When I tap ♻️ (refresh)
  Then I see an error notification
  When I click on the notification
  Then it takes me to Fill Blank Form
  And I see an error badge on the refresh button

• [x] Given I've set up Match Exactly and have navigated to Fill Blank Forms
  And my credentials have been revoked/changed (a 401)
  When I tap ♻️ (refresh)
  Then I see an error notification
  When I click on the notification
  Then it takes me to Fill Blank Form
  And I see a prompt to fix credentials (like Get Blank Forms)
  And I see an error badge on the refresh button

• [x] Given I've set up Match Exactly and have navigated to Fill Blank Forms
  When I tap ♻️ (refresh)
  And then ANY (includes form list, manifest or download) of the network requests fail
  Then I see an error notification
  When I click on the notification
  Then it takes me to Fill Blank Form
  And I see an error badge on the refresh button

• [x] Given I've set up Match Exactly and have navigated to Fill Blank Forms
  When I tap ♻️ (refresh)
  And then there is file system error
  Then I see an error notification
  When I click on the notification
  Then it takes me to Fill Blank Form
  And I see an error badge on the refresh button

• [x] Given I've had an error
  When I navigate to Fill Blank Forms
  When I tap ♻️ (refresh)
  And the sync succeeds
  Then the error state disappears

• [x] Given I've set up Match Exactly and have navigated to Fill Blank Forms
  And the server returns bad form list and or manifest responses
  When I tap ♻️ (refresh)
  Then I see an error notification
  When I click on the notification
  Then it takes me to Fill Blank Form
  And I see an error badge on the refresh button

• [x] All errors while syncing in the background should result in same notification and error badge

[seadowg]

@lognaturel good to get your thoughts here. The copy is straight off the top of my head so could definitely use some work but I wanted to focus on capturing the different error states and how we present them. My logic here was to continue to use a dialog for most errors as they involve trying to push the enumerator to check details/contact someone. For the "sync error" a banner felt more appropriate as we want to make sure we keep alerting them to the problem but without blocking them.

[lognaturel]

Here's a more radical option: if anything at all goes wrong during an update, the refresh button gets a red badge with an exclamation mark in it. This gives the user a visual cue to try again or just that something bad has happened. More descriptive error state information is pushed to a notification, with an exception for a 401 which presents an auth dialog.

Regardless of where error states are displayed, I think we should have a common sentence that we include for all problems that are related to the specific data collection project rather than the app. I'm thinking something like "If you keep having this problem, please contact the person who gave you the server address or configured ODK Collect." This avoids more specific language like "project administrator" which may not mean anything to users (e.g. you scanned a QR code at a zoo and the server is down, you have no idea who administrates the project but you can find the person who passed around the code).

I know you're trying to keep the criteria around what the user experiences but I want to be a little more precise around exceptions and status codes. Maybe once we've reached some conclusions you can update the criteria.

Client has no internet: same toast as Get Blank Form ("No network connection available"). It looks like all the Google apps do this and it is a helpful curtesy since it's typically so easy to recover from.

UnknownHostException: If it's not a notification, seems right to be a dialog. "Collect can't contact the server '%s'. Are you sure you entered it correctly? If you keep..." I think it's important to include the URL that was attempted.

401: I think this should show the same auth dialog used by the FormDownloadListActivity. Some folks (rightfully) don't want to save their credentials in Collect and just enter them in each time they communicate with the server.

Seems right for all network request failures to result in the same text. A Material Banner doesn't feel right to me because there aren't really actions to display (the refresh button is already right there). If it's not a notification, I think a toast would be appropriate. "Syncing with server failed. Please try again." -> "Updating forms failed. Please try again. If you keep..." We don't use the word "sync" elsewhere so I think we should avoid it.

Do you think it's essential for parsing errors to get a separate message? I think it would be ok to use the same message as with failed network requests. I agree that it's different from a troubleshooting standpoint because this is more likely to be a permanent kind of error rather than something situational. However, it seems vanishingly unlikely that it would happen for a standard server. And if it does happen then what we want is a forum post and we could debug from there. It's not clear a project administrator could do anything even if they knew it was a parse error.

There's also another class of error which would be something going wrong when saving an update from the server, right? Something like the device being out of space, something weird happening when writing to the db, etc. I could go for the same generic message for these as well ("Updating forms failed. Please..."). I think most of these are likely to go away with a retry. Running out of space is the only one that I can think of needing action outside Collect.

I was previously imagining that it would be helpful to indicate when the last successful update happened. I no longer think it really matters. Something like the red badge I mentioned previously would be enough to indicate that the previous update failed in some way. If an update happened successfully and there were forms to download, those form's added date/time stamp would indicate when the update happened.

[seadowg]

I've updated the acceptance to specific showing the errors in a Notification rather than in a banner and or dialog

[lognaturel]

We are proposing adding three strings:

• "Collect can't contact the server '%s'. Are you sure you entered it correctly?" - new special case for an unknown host.
• "Updating forms failed. Please try again." - message for any non-401 unhappy network response or error during the saving process.
• "If you keep having this problem, please contact the person who gave you the server address or configured ODK Collect." - generic message we can add whenever there's an issue that has to do with the server or project configuration rather than Collect itself.

See https://github.com/getodk/collect/issues/3959#issuecomment-662100479 for latest.

Any comments on the wording? I'd like to get a PR in early next week adding those strings so that they can be translated.

I think we should log analytics events for successful match exactly attempts and for the generic unhappy case (not unknown host and not 401). If the unhappy case is uncommon, then keeping the generic error message is sufficient. If there are frequently failures then we can see what we can learn about their cause and how we can help users recover. @seadowg, something to add to acceptance criteria either here or at https://github.com/getodk/collect/issues/3958?

[seadowg]

@lognaturel I've added a note to the analytics issue (https://github.com/getodk/collect/issues/3958) to include error types with the failure analytics.

[yanokwa]

I'd propose these changes...

• "Collect can't reach the server at '%s'. Did you enter the URL correctly?"
• "Form update failed. Please try again." only for form update issues.
• "Form download failed. Please try again." only for get blank form issues.
• "If you keep having this problem, report it to the person who asked you to collect data."

[seadowg]

Cool so I'll add 4 strings:

• generic_network_error: "If you keep having this problem, report it to the person who asked you to collect data."
• form_download_error: "Form download failed. Please try again."
• form_update_error: "Form update failed. Please try again."
• unknown_host_error: "Collect can't reach the server at '%s'. Did you enter the URL correctly?"

[seadowg]

Actually starting to put this in and realized that really we need strings for titles and body (for notifications or the existing dialogs). I'm going to adapt @yanokwa's strings so that we have situational titles: ("Form download failed", "Form update failed") etc and then the body is generic_network_error other than the unknown_host_error case (and auth but that's a different flow).