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 messages and user feedback. #150

Merged
merged 6 commits into from
Oct 29, 2017
Merged

Improve error messages and user feedback. #150

merged 6 commits into from
Oct 29, 2017

Conversation

stephank
Copy link
Member

@stephank stephank commented Oct 27, 2017

This PR is on top of #149.

There are a bunch of changes to error handling here:

  • I've added BrokerError::SessionExpired, because I expect it to be a very common case, for which we can now give better feedback. The other classes of BrokerError::Input are all integration issues, and can use the generic template.
  • I've added BrokerError::ProviderInput, which covers some of the previous cases for BrokerError::Provider and BrokerError::Input. This is specifically for errors related to the request input (thus 400), not provider communication (still 503). All BrokerError::Input is now related to the RP, while all BrokerError::Provider* is related to the provider. Error messages reflect this.
  • Internal errors and rate limiting now no longer redirect back to the RP, and always display a message.
  • Internal errors are now logged with a reference number, which is also visible on the error page. This number is also z-base-32, so it can also be read on the phone, for example.
  • Error pages are now translated.

@onli Can you push a commit here with German translations for the new messages, if you can spare moment? :)

Example Dutch 'session expired' page:
screen shot 2017-10-27 at 15 24 55

Example Dutch 'internal error' page:
screen shot 2017-10-27 at 15 24 04

@portier portier deleted a comment from coveralls Oct 27, 2017
@coveralls
Copy link

Coverage Status

Changes Unknown when pulling a3786c0 on featErrors into ** on master**.

@onli
Copy link
Member

onli commented Oct 29, 2017

Hi @stephank. Very nice. I added the german translation and approved the PR.

@coveralls
Copy link

Coverage Status

Changes Unknown when pulling 763b177 on featErrors into ** on master**.

@stephank stephank merged commit b0388c6 into master Oct 29, 2017
@stephank stephank deleted the featErrors branch October 29, 2017 16:55
@stephank
Copy link
Member Author

Awesome, thanks! 😃

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

Successfully merging this pull request may close these issues.

None yet

3 participants