[
https://issues.apache.org/jira/browse/SHINDIG-1180?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12758911#action_12758911
]
Jon Weygandt commented on SHINDIG-1180:
---------------------------------------
Code review at: http://codereview.appspot.com/123044
> makeRequest does not properly handle server errors, plus standardizing error
> handling
> -------------------------------------------------------------------------------------
>
> Key: SHINDIG-1180
> URL: https://issues.apache.org/jira/browse/SHINDIG-1180
> Project: Shindig
> Issue Type: Bug
> Components: Javascript
> Environment: all
> Reporter: Jon Weygandt
> Priority: Minor
> Attachments: fix-1180-bug.patch
>
>
> When doing a makeRequest, if the remote server returns an error it is not
> properly handled. The XMLHttpRequest has a status code of 200, and data is
> present in the "don't be evil" json object. "rc" is set to an error code, and
> text is set to some value (generally an HTML page) returned by the server.
> Inside of io.js the overall 200 code passes through hadError; in
> transformResponseData, errors is set to [] and rc is set to the value. Then
> it tries to parse the text, which is probably not JSON and throws an
> exception.
> To make matters worse, there is no real documented standard for the behavior
> of makeRequest. See the thread:
> http://groups.google.com/group/opensocial-and-gadgets-spec/browse_thread/thread/60f73b51799fd315#
> .
> This patch
> *) fixes the breakage in transformResponseData,
> *) cleans up all the error handling so it conforms with the OpenSocial
> thread.
> *) The errors array is now "<code> <msg>", rather than "<msg> <code>" per
> open social article. It should be noted that the discussion proposed a more
> human readable form, that might eliminate the <code> from the message. I'll
> change it again, if that happens.
> *) fixes jsonrpccontainer.js so that it uses rc rather than errors.
> *) fixes makeRequestText.xml (the numeric assertEquals test matches the
> comment string!)
> *) updates iotests.js to properly deliver rc in the tests, plus add a test to
> demonstrate the original bug.
> This change could cause issues with gadget authors, simply because there was
> no real specification. As long as users were using simple techniques, like
> ignoring errors, looking at the errors array as human readable string,
> looking at rc when present, try/catch things are OK. If like
> jsonrpccontainer.js, they depended upon the human readable string in errors
> to be of a specific format, they will break with this change.
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
