search

tidepool-org / hub

[DEPRECATED] Central storage for Tidepool planning and issue tracking.
2 stars 2 forks source link

Show error message when server calls fail (500), redirect to login when auth fails (401) #97

Closed nicolashery closed 9 years ago

nicolashery commented 10 years ago

Add to both Blip and Clamshell.

When auth fails (401), probably show an error message with link to go back to login. Don't redirect immediately, as might be surprising.

Possible copy (suggestions welcome, links in brackets):

nicolashery commented 10 years ago

@HowardLook, @brandonarbiter Any thoughts on the error messages above we would display in Blip and Clamshell if an error occurred while talking to the platform? Thanks! :)

brandonarbiter commented 10 years ago

Looping in @skrugman

cheddar commented 10 years ago

For 401, instead of making like a 2-step modal process that requires user action to get to the thing we want them to do (log in). Why not have the modal include the login dialog?

Though, for logging in, we will hopefully be separating that from blip in the not-too-distant future, so it's probably best to think of what we want that experience to be and make this do whatever that is.

Btw, on error messages that we show, it will be relatively common for a user to screen shot it and send that to us. In this case, it is actually quite helpful to have some text that will only really be decipherable by us included in the screen shot (it should be discrete enough to not bother the user though).

brandonarbiter commented 10 years ago

@nicolashery @jh-bate Here's updated text to use. @skrugman , @HowardLook feel free to modify.

Backend error (500): "Sorry! Something went wrong. It's our fault, not yours. We're going to go investigate. For the time being, go ahead and [refresh your browser]."

Auth error (401): "Yikes! It looks like you got logged out. We're going to go figure out what happened. In the meantime, [click here to log back in]."

cheddar commented 10 years ago

"We're going to go figure out what happened" on the 401 is misleading. We won't look into why they got logged out, getting logged out is a relatively common thing that is done for security purposes more than anything.

That is, we should expect everyone that regularly uses Blip to experience the "you got logged out" flow on a regular basis (more than one a week).

skrugman commented 10 years ago

What about either of these for Auth error (401)?

"You were signed into blip for a long time without doing anything so we logged you out. [click here] to log back in"

"To keep your data safe we logged you out - to log back in [click here]."

On Fri, May 16, 2014 at 3:27 PM, cheddar notifications@github.com wrote:

"We're going to go figure out what happened" on the 401 is misleading. We won't look into why they got logged out, getting logged out is a relatively common thing that is done for security purposes more than anything.

That is, we should expect everyone that regularly uses Blip to experience the "you got logged out" flow on a regular basis (more than one a week).

— Reply to this email directly or view it on GitHubhttps://github.com/tidepool-org/hub/issues/97#issuecomment-43330553 .

Sara Krugman Lead Interaction Designer

Tidepool An open source, not-for-profit effort to build an open data platform and better applications that reduce the burden of Type 1 Diabetes and accelerate the commercialization of closed-loop systems.

Phone : +45 42 74 68 17 Email : sara@tidepool.org Web : Tidepool.org http://tidepool.org/

nicolashery commented 10 years ago

@skrugman I like that second one, will use that. Thanks!

brandonarbiter commented 10 years ago

Me too. I'd changed hyphon to a period to make it two sentences, see below.

"To keep your data safe we logged you out - to log back in [click here]."

"To keep your data safe we logged you out. To log back in [click here]."

Brandon Arbiter VP, Product and Business Development

Tidepool An open source, not-for-profit effort to build an open data platform and better applications that reduce the burden of Type 1 Diabetes and accelerate the commercialization of closed-loop systems.

Phone : 917 536 0505 Email : brandon@tidepool.org Web : Tidepool.org http://tidepool.org/

On Wed, May 21, 2014 at 8:21 AM, Nicolas Hery notifications@github.comwrote:

@skrugman https://github.com/skrugman I like that second one, will use that. Thanks!

— Reply to this email directly or view it on GitHubhttps://github.com/tidepool-org/hub/issues/97#issuecomment-43769457 .

nicolashery commented 10 years ago

Good idea. I'd actually push it further and make the whole second sentence the link, see pic?

screen shot 2014-05-21 at 6 11 21 pm

nicolashery commented 10 years ago

And a preview of what a "server error" would look like...

screen shot 2014-05-21 at 6 28 07 pm

brandonarbiter commented 10 years ago

I love "To keep your data safe..."

Brandon Arbiter VP, Product and Business Development

Tidepool An open source, not-for-profit effort to build an open data platform and better applications that reduce the burden of Type 1 Diabetes and accelerate the commercialization of closed-loop systems.

Phone : 917 536 0505 Email : brandon@tidepool.org Web : Tidepool.org http://tidepool.org/

On Wed, May 21, 2014 at 9:12 AM, Nicolas Hery notifications@github.comwrote:

Good idea. I'd actually push it further and make the whole second sentence the link, see pic?

[image: screen shot 2014-05-21 at 6 11 21 pm]https://cloud.githubusercontent.com/assets/1306536/3043053/b0bb5250-e102-11e3-9571-9ad99a88b0f8.png

— Reply to this email directly or view it on GitHubhttps://github.com/tidepool-org/hub/issues/97#issuecomment-43778283 .

brandonarbiter commented 10 years ago

I believe @Cheddar suggested against including the words "We're going to go investigate."

Brandon Arbiter VP, Product and Business Development

Tidepool An open source, not-for-profit effort to build an open data platform and better applications that reduce the burden of Type 1 Diabetes and accelerate the commercialization of closed-loop systems.

Phone : 917 536 0505 Email : brandon@tidepool.org Web : Tidepool.org http://tidepool.org/

On Wed, May 21, 2014 at 9:31 AM, Nicolas Hery notifications@github.comwrote:

And a preview of what a "server error" would look like...

[image: screen shot 2014-05-21 at 6 28 07 pm]https://cloud.githubusercontent.com/assets/1306536/3043341/4833c390-e105-11e3-8a52-c37e91efb2fb.png

— Reply to this email directly or view it on GitHubhttps://github.com/tidepool-org/hub/issues/97#issuecomment-43781276 .

nicolashery commented 10 years ago

For 401 (= expired auth) right? We don't have it for that (yellow message)...

cheddar commented 10 years ago

Yeah, investigations of 500 might happen. 401s won't.

I'm guessing the suggestion to not require an extra click from the user to give them a login prompt was veto'd?

skrugman commented 10 years ago

That looks good - I would make the colors consistent with style guide - but we are chatting about that tomorrow :)

On Wed, May 21, 2014 at 6:12 PM, Nicolas Hery notifications@github.comwrote:

Good idea. I'd actually push it further and make the whole second sentence the link, see pic?

[image: screen shot 2014-05-21 at 6 11 21 pm]https://cloud.githubusercontent.com/assets/1306536/3043053/b0bb5250-e102-11e3-9571-9ad99a88b0f8.png

— Reply to this email directly or view it on GitHubhttps://github.com/tidepool-org/hub/issues/97#issuecomment-43778283 .

Sara Krugman Lead Interaction Designer

Tidepool An open source, not-for-profit effort to build an open data platform and better applications that reduce the burden of Type 1 Diabetes and accelerate the commercialization of closed-loop systems.

Phone : +45 42 74 68 17 Email : sara@tidepool.org Web : Tidepool.org http://tidepool.org/

skrugman commented 10 years ago

For the "server error" do we need to show the "original error message"?

On Wed, May 21, 2014 at 6:31 PM, Nicolas Hery notifications@github.comwrote:

And a preview of what a "server error" would look like...

[image: screen shot 2014-05-21 at 6 28 07 pm]https://cloud.githubusercontent.com/assets/1306536/3043341/4833c390-e105-11e3-8a52-c37e91efb2fb.png

— Reply to this email directly or view it on GitHubhttps://github.com/tidepool-org/hub/issues/97#issuecomment-43781276 .

Sara Krugman Lead Interaction Designer

Tidepool An open source, not-for-profit effort to build an open data platform and better applications that reduce the burden of Type 1 Diabetes and accelerate the commercialization of closed-loop systems.

Phone : +45 42 74 68 17 Email : sara@tidepool.org Web : Tidepool.org http://tidepool.org/

nicolashery commented 10 years ago

It is a bit "ugly" @skrugman I agree, but it's something that @cheddar suggested because users are likely to take screenshots and send it to us, and I do agree that's a good idea. It gives us a bit more information, and I expect the users will largely ignore it. They'll be just like "eh, it broke", screenshot it and close the window. No?

skrugman commented 10 years ago

If it's really helpful information then I suggest to make it as small as possible and still be readable and light grey.

On May 22, 2014, at 2:03 PM, Nicolas Hery notifications@github.com wrote:

It is a bit "ugly" @skrugman I agree, but it's something that @cheddar suggested because users are likely to take screenshots and send it to us, and I do agree that's a good idea. It gives us a bit more information, and I expect the users will largely ignore it. They'll be just like "eh, it broke", screenshot it and close the window. No?

— Reply to this email directly or view it on GitHub.

kentquirk commented 10 years ago

The server errors happen only on serious problems -- it means something has gone wrong in our operations and the maximum information we can get is helpful. So please make sure that we have metrics events for these sorts of errors. And @skrugman -- given that it's important and should be quite rare, please let's not overly suppress the extra information.

brandonarbiter commented 10 years ago

@kentquirk should we build a Kissmetrics report around server errors?

Sent from my iPhone

On May 22, 2014, at 6:08 AM, Kent Quirk notifications@github.com wrote:

The server errors happen only on serious problems -- it means something has gone wrong in our operations and the maximum information we can get is helpful. So please make sure that we have metrics events for these sorts of errors. And @skrugman -- given that it's important and should be quite rare, please let's not overly suppress the extra information.

— Reply to this email directly or view it on GitHub.

kentquirk commented 10 years ago

Yes, once we have errors logging to KM.

brandonarbiter commented 10 years ago

@kentquirk I don't see a trello card for error logging in Kissmetrics. Do we have a github issue for it?

Sent from my iPhone

On May 22, 2014, at 7:48 AM, Kent Quirk notifications@github.com wrote:

Yes, once we have errors logging to KM.

— Reply to this email directly or view it on GitHub.

skrugman commented 10 years ago

@kentquirk Even if its rare and important - it is of no use to the person using the application, its only of use to you guys who are trying to see why it went wrong. Is there an issue with minimizing for the user but still legible for you all?

On Thu, May 22, 2014 at 3:08 PM, Kent Quirk notifications@github.comwrote:

The server errors happen only on serious problems -- it means something has gone wrong in our operations and the maximum information we can get is helpful. So please make sure that we have metrics events for these sorts of errors. And @skrugman https://github.com/skrugman -- given that it's important and should be quite rare, please let's not overly suppress the extra information.

— Reply to this email directly or view it on GitHubhttps://github.com/tidepool-org/hub/issues/97#issuecomment-43884880 .

Sara Krugman Lead Interaction Designer

Tidepool An open source, not-for-profit effort to build an open data platform and better applications that reduce the burden of Type 1 Diabetes and accelerate the commercialization of closed-loop systems.

Phone : +45 42 74 68 17 Email : sara@tidepool.org Web : Tidepool.org http://tidepool.org/

nicolashery commented 10 years ago

I can make the original error text more subtle when I work on the colors. @skrugman see the email I sent you on that subject, let me know if there's anything unclear. Thanks!

cheddar commented 10 years ago

@nicolashery is there private content to the discussion on what colors to use? As someone not involved but trying to keep track of what's going on, a super secret email about something that is hard to imagine privacy issues around (i.e. colors), makes me feel like I'm not important enough to be involved in this project.

nicolashery commented 10 years ago

@cheddar I just put our super secret conversation here for all to see: https://github.com/tidepool-org/blip/issues/85

cheddar commented 10 years ago

@nicolashery Thanks! As I expected it's not much, but I feel more in the loop now :).

kentquirk commented 10 years ago

To be clear -- the PR resolving this issue should include a metrics call. If it uses the code that was created for Clamshell, it should be sufficient.

jh-bate commented 10 years ago

It does make a metrics call via logging of the error to the platform-client

On Fri, May 23, 2014 at 7:44 AM, Kent Quirk notifications@github.comwrote:

To be clear -- the PR resolving this issue should include a metrics call.

— Reply to this email directly or view it on GitHubhttps://github.com/tidepool-org/hub/issues/97#issuecomment-43935118 .