You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@shindig.apache.org by "Jon Weygandt (JIRA)" <ji...@apache.org> on 2009/09/23 23:35:18 UTC
[jira] Created: (SHINDIG-1180) makeRequest does not properly handle
server errors, plus standardizing error handling
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.
[jira] Commented: (SHINDIG-1180) makeRequest does not properly
handle server errors, plus standardizing error handling
Posted by "Paul Lindner (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/SHINDIG-1180?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12758937#action_12758937 ]
Paul Lindner commented on SHINDIG-1180:
---------------------------------------
This looks a little bit like SHINDIG-848 -- Jon wdyt?
> 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
> 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.
[jira] Commented: (SHINDIG-1180) makeRequest does not properly
handle server errors, plus standardizing error handling
Posted by "Paul Lindner (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/SHINDIG-1180?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12776645#action_12776645 ]
Paul Lindner commented on SHINDIG-1180:
---------------------------------------
lgtm
> 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
> Assignee: John Hjelmstad
> 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.
[jira] Reopened: (SHINDIG-1180) makeRequest does not properly
handle server errors, plus standardizing error handling
Posted by "John Hjelmstad (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/SHINDIG-1180?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]
John Hjelmstad reopened SHINDIG-1180:
-------------------------------------
Assignee: John Hjelmstad
Reopening for follow-up tracking.
> 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
> Assignee: John Hjelmstad
> 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.
[jira] Commented: (SHINDIG-1180) makeRequest does not properly
handle server errors, plus standardizing error handling
Posted by "Jon Weygandt (JIRA)" <ji...@apache.org>.
[ 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.
[jira] Updated: (SHINDIG-1180) makeRequest does not properly handle
server errors, plus standardizing error handling
Posted by "Paul Lindner (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/SHINDIG-1180?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]
Paul Lindner updated SHINDIG-1180:
----------------------------------
Affects Version/s: 1.1-BETA2
> 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
> 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.
[jira] Updated: (SHINDIG-1180) makeRequest does not properly handle
server errors, plus standardizing error handling
Posted by "Jon Weygandt (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/SHINDIG-1180?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]
Jon Weygandt updated SHINDIG-1180:
----------------------------------
Attachment: fix-1180-bug.patch
The patch file with changes
> 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.
[jira] Commented: (SHINDIG-1180) makeRequest does not properly
handle server errors, plus standardizing error handling
Posted by "Jon Weygandt (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/SHINDIG-1180?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12759172#action_12759172 ]
Jon Weygandt commented on SHINDIG-1180:
---------------------------------------
It is similar, but that change was not done for various reasons. Hope I addressed those issues.
I did make some changes and will attach a new patch file, and have also uploaded the new patch to appspot.
I examined the Status Codes: http://www.w3.org/Protocols/rfc2616/rfc2616-sec10.html, and altered the code to accept all 200 responses as legitimate. Added a unit test for a 203 response code.
I modified the if/else block in transformResponseData to only parse JSON/XML/DOM for 2xx responses. I went back and forth on that because of unit test failures. I think the if/elseif is the right way to go.
A code inspection of makeRequest shows that FetchResponseUtils will always put an "rc" in it, so we should be OK for real makeRequests.
Code like jsonrpccontainer users core.io in non-publicly documented ways. A review of OpenSocial RPC spec documents the return value of an RPC call when successful to exclude "rc". Thus in transformResponseData rc defaults to 200 if absent. It allows the core.io code to work properly.
As part of the original patch changes were made to jsonrpccontainer so that its API behavior is preserved with these changes.
I did a mvn build against r816766 and it was fine.
> 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
> 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.
[jira] Updated: (SHINDIG-1180) makeRequest does not properly handle
server errors, plus standardizing error handling
Posted by "Jon Weygandt (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/SHINDIG-1180?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]
Jon Weygandt updated SHINDIG-1180:
----------------------------------
Attachment: fix-1180-bug-v2.patch
To supersede the previous patch.
> 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
> 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.
[jira] Resolved: (SHINDIG-1180) makeRequest does not properly
handle server errors, plus standardizing error handling
Posted by "Paul Lindner (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/SHINDIG-1180?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]
Paul Lindner resolved SHINDIG-1180.
-----------------------------------
Resolution: Fixed
Fix Version/s: 1.1-BETA4
looks good.
Of course everyone should start using osapi.http at some point!
> 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.
[jira] Commented: (SHINDIG-1180) makeRequest does not properly
handle server errors, plus standardizing error handling
Posted by "John Hjelmstad (JIRA)" <ji...@apache.org>.
[ 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.
[jira] Updated: (SHINDIG-1180) makeRequest does not properly handle
server errors, plus standardizing error handling
Posted by "Paul Lindner (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/SHINDIG-1180?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]
Paul Lindner updated SHINDIG-1180:
----------------------------------
Fix Version/s: (was: 1.1-BETA4)
Affects Version/s: (was: 1.1-BETA2)
1.1-BETA3
> 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-BETA3
> Environment: all
> Reporter: Jon Weygandt
> Assignee: John Hjelmstad
> Priority: Minor
> 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.