-
Notifications
You must be signed in to change notification settings - Fork 493
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
Fix inconsistent cross origin error handling #667
Conversation
@@ -4,6 +4,7 @@ var windowHelper = require('../helper/window'); | |||
var objectHelper = require('../helper/object'); | |||
var RequestBuilder = require('../helper/request-builder'); | |||
var WebMessageHandler = require('./web-message-handler'); | |||
var responseHandler = require('../helper/response-handler'); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
what convention are you following by using a lowercase for the first letter and a capital one for the WebMessageHandler
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
WebMessageHandler is a class, responseHandler is a function
expect(err).to.be.eql({ error: 'any_error', error_description: 'any error' }); | ||
expect(err).to.be.eql({ | ||
original: { error: 'any_error', error_description: 'any error' }, | ||
code: 'any_error', |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I know that it's different any_error
to any error
, but can you please change one of them to something more visibly different? i.e. code: 'any_error', description: 'a long error description'
error_description: '{"some":"error"}' | ||
original: { | ||
error: 'request_error', | ||
error_description: '{"some":"error"}' |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
are you sure this particular error description is an object?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
it's a string, not an object.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We weren't applying the error mapping that we do in other errors.