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

Improve error handling when Lock can't fetch client/tenant settings or language file #958

Closed
luisrudge opened this issue Mar 29, 2017 · 12 comments · Fixed by #961
Closed
Assignees

Comments

@luisrudge
Copy link
Contributor

luisrudge commented Mar 29, 2017

Currently, as pointed out by #957 and a couple of internal tickets, when Lock can't fetch the client/tenant settings or the language file, we just throw an error. We should, instead, show an error in the lock widget itself (like when fetching ssodata fails).
We should also increase the timeout to fetch data remotely.

@selaux
Copy link
Contributor

selaux commented Mar 29, 2017

I adapted the code a little to distinguish the type of errors thrown: holidaycheck@9794a8a, but in general as long as everything is loaded via a script tag, it will be quite hard to log proper errors. E.g. a 500 http status code from the server will be the same as the user not having a working internet connection.

@selaux
Copy link
Contributor

selaux commented Mar 29, 2017

Also the fixed timeout of 5000ms might be too low for mobile devices, but we will verify that with the better error reporting.

@lo1tuma
Copy link

lo1tuma commented Mar 29, 2017

Generally I would say a retry approach would be a good idea in case of timeout errors, because especially on mobile device there is a high chance of having a flaky network connection. But due to the fact lock uses script tags it might be quite hard to implement.

@brassier
Copy link

This first change may help with error details, but it won't address the hard crashing after 5 seconds. Can something less drastic happen after the fixed timeout? Not sure if the error on the lock widget is possible when we can't load the lock script in this case?

Is a configurable timeout an option?

@luisrudge
Copy link
Contributor Author

The goal is to show a nice error message in lock itself and increase the timeout.

@luisrudge
Copy link
Contributor Author

@brassier to answer your question more directly: there's no plans to add a configurable timeout at the moment.

@dariobanfi
Copy link
Contributor

Is the fetching of this data even necessary? For example couldn't I initialise the lock with the proper translations and client settings so that it doesn't require any initial loading? @luisrudge

@luisrudge
Copy link
Contributor Author

@dariobanfi we have this in the backlog as a high priority. I can't give any estimates, but it's coming!

@selaux
Copy link
Contributor

selaux commented Mar 31, 2017

I think we can already remove every request except for client data right now by setting sso: false and providing the whole language dictionary and not set a specific language. So we are quite close already 🙂 (at least for our use-case)

@luisrudge
Copy link
Contributor Author

Yeah. We'll add the option to add your connections so you don't have to do that request.

@jfmyers
Copy link

jfmyers commented Apr 6, 2017

Has there been any update on this? What are people doing in the interim to handle for their users?

@luisrudge
Copy link
Contributor Author

This was fixed here and will be in the next release (next monday). In the interim, you can install from that branch or wait a few days

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

Successfully merging a pull request may close this issue.

6 participants