Skip to content
This repository has been archived by the owner on Aug 22, 2019. It is now read-only.

issue#144 throwing proper error message for badge assertion in issuer api #159

Merged
merged 2 commits into from May 10, 2012
Merged

issue#144 throwing proper error message for badge assertion in issuer api #159

merged 2 commits into from May 10, 2012

Conversation

pradeepmurugesan
Copy link
Contributor

No description provided.

@travisbot
Copy link

This pull request passes (merged 4237178 into e1998fd).

@@ -74,7 +74,7 @@
<div id="inaccessible-template">
<div class="alert alert-error">
<a class="close">×</a>
An assertion URL could not be retrieved.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm a little nervous to rely solely on the error message here, unless we're certain that all error responses have sensible messages. I'd either like to beef up the testing around that, or add some sort of "We've encountered the following problem:" text to preface error.message. At least that way if it's blank or undecipherable, it's not just totally empty.

@stenington
Copy link
Contributor

Hey @pradeepmurugesan, this looks pretty good. There are two little fixes I'd think would be good to add before merging, but I think I can leave the request open and you can add a commit to it.

@travisbot
Copy link

This pull request passes (merged 46cf0e6 into e1998fd).

@pradeepmurugesan
Copy link
Contributor Author

@stenington : Fixed as per the comments
@brianloveswords : we already discussed about adding the error message as the reason, but I feel this is more convincing like adding the reason as INACCESSIBLE and the exception message as a new component "message".

stenington added a commit that referenced this pull request May 10, 2012
issue#144 throwing proper error message for badge assertion in issuer api
@stenington stenington merged commit 65e69b8 into mozilla:development May 10, 2012
@stenington
Copy link
Contributor

Thanks!

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

Successfully merging this pull request may close these issues.

None yet

3 participants