[
https://issues.apache.org/jira/browse/SHINDIG-1180?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12776591#action_12776591
]
John Hjelmstad commented on SHINDIG-1180:
-----------------------------------------
In testing a build with this patch, we found that 3xx responses are also
classified as errors. I'd argue this is shouldn't be so.
We do want to avoid parsing resp.text in 3xx situations however, as it will
often be random HTML that may break on params.CONTENT_TYPE = "DOM", "FEED", or
"JSON".
The following CL implements a fix:
http://codereview.appspot.com/152070
> 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
> Affects Versions: 1.1-BETA2
> Environment: all
> Reporter: Jon Weygandt
> Priority: Minor
> Fix For: 1.1-BETA4
>
> Attachments: fix-1180-bug-v2.patch, 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.
