Documenting the data object in the callback for makeRequest() for 1.0

[Jon]

Assuming the community does not decide to kill off makeRequest,
perhaps we could document what the object is that is passed to the
callback. Of particular interest is error handling.

Current reference documents would be:
*) V0.9 of the specification, section 5.2.2.3.5 (which says little
about the data)
*) An OpenSocial article: http://wiki.opensocial.org/index.php?title=Introduction_to_makeRequest
*) From the pipelining specification <os:HttpRequest...>.
*) Shindig source code, io.js - I think there may be some
inconsistency in this implementation.

The OpenSocial article, in the section "Response Structure" has a good
definition, and at a minimum that should be added to the API
reference.

One question: Shindig adds the parameter "rc", and os:HttpRequest has
the equivalent parameters "status" - should these be included in the
core API?

Thanks,
Jon

[Adam Winer]

On Tue, Sep 15, 2009 at 10:41 AM, Jon <jon.we...@gmail.com> wrote:
>
> Assuming the community does not decide to kill off makeRequest,
> perhaps we could document what the object is that is passed to the
> callback. Of particular interest is error handling.
>
> Current reference documents would be:
> *) V0.9 of the specification, section 5.2.2.3.5 (which says little
> about the data)
> *) An OpenSocial article: http://wiki.opensocial.org/index.php?title=Introduction_to_makeRequest
> *) From the pipelining specification <os:HttpRequest...>.

This is not a reference. gadgets.io.makeRequest() is entirely
independent of os:HttpRequest. gadgets.io.makeRequest() was an ad hoc
data structure. os:HttpRequest started from sorta-scratch to make it
consistent with JSON-RPC, and fix problems with the original
structure.

> *) Shindig source code, io.js - I think there may be some
> inconsistency in this implementation.
>
> The OpenSocial article, in the section "Response Structure" has a good
> definition, and at a minimum that should be added to the API
> reference.
>
> One question: Shindig adds the parameter "rc", and os:HttpRequest has
> the equivalent parameters "status" - should these be included in the
> core API?

Ignore os:HttpRequest here. So "rc" yes, "status" no.

As a first draft, I'd say "do what Shindig does for io.js", though
we'd of course need to weigh that against any differences in other
implementations.

-- Adam

>
> Thanks,
> Jon
> >
>

[Paul Lindner]

Another ugly problem is the current status of osapi.http in shindig..

I played around with this a few days ago and to my surprise the 0.9 documented way of making requests:

  osapi.http.get("http://example.com", {param: "value"})

is broken.  Instead you have to do this:

  osapi.http.get({href: "http://example.com", param: "value"})

IMO this is better since it maps one-to-one with the json-rpc endpoint, however someone who picks up 0.9 and tries this will be in for some serious confusion.  (The error message thrown is particularly ugly too, a class cast exception...)

So..  fix the spec or fix the implementation?

[Arne Roomann-Kurrik]

This came up before when I started poking around with osapi.http.  I believe the consensus was fix the spec, although I forget if that meant patching 0.9 or just correcting things in 1.0.

~Arne

[Adam Winer]

Yes, that was the consensus. The spec is just wrong, and this should
be an errata for 0.9.

-- Adam

[Jon]

The reference to os:HttpRequest was that both an "HTTP Status
Code" (an integer), and a text message are available. Shindig tries to
make both available. The OpenSocial article, and as far as I can tell
osapi.http only has the text message. Because these are to be an HTTP
request, the actual "HTTP Status Code" has value.

So I propose the following wording for section "5.2.2.3.5 The callback
parameter". BTW - I'm not very familiar with OAuth, and simply copied
the text for those parameters, but they might need some work as well.

The makeRequest() callback function is passed a javascript object with
the following fields:

"rc"
This will contain the HTTP Status Code (http://www.w3.org/
Protocols/rfc2616/rfc2616-sec10.html) for the request. This field will
always be present.
"errors"
This will contain an array of any errors that occurred when making
this request. For example, if a 500 error occurred in the request:
[ "500 error" ]. If rc is 200, this field if present must be a zero
length array. If rc is not 200, this field must be present, and
contain at least one element.
"text"
This will return the unparsed text of the response.
"data"
This will contain a different type of data depending on the type
of request that was made, and is only present if rc is 200. This is
controlled by the parameter gadgets.io.RequestParameters.CONTENT_TYPE,
with values enumerated in gadgets.io.ContentType.

There may be several additional OAuth-specific fields:

"oauthApprovalUrl"
If this value is specified, the user needs to visit an external
page to approve the gadget's request to access data. Use of a pop-up
window to direct the user to the external page is recommended. Once
the user has approved access, the gadget can repeat the makeRequest
call to retrieve the data.
"oauthError"
If this value is specified, it indicates an OAuth-related error
occurred. The value will be one of a set of string constants that can
be used for programmatically detecting errors. The constants are
undefined for opensocial-0.8, but implementers should attempt to agree
on a set of useful constant values for standardization in
opensocial-0.9.
"oauthErrorText"
If this value is specified, it indicates an OAuth-related error
occurred. The value is free-form text that can be used to provide
debugging information for gadget developers.


On Sep 15, 5:53 pm, Adam Winer <awi...@gmail.com> wrote:

[Tim Moore]

This looks pretty good, but there are a few sections that I think need
more detail.

On Sep 16, 11:38 am, Jon <jon.weyga...@gmail.com> wrote:
> "errors"
>     This will contain an array of any errors that occurred when making
> this request. For example, if a 500 error occurred in the request:
> [ "500 error" ]. If rc is 200, this field if present must be a zero
> length array. If rc is not 200, this field must be present, and
> contain at least one element.

Needs to define what the elements of the array are. I'd also
recommend clarifying what the intended use of these errors are (i.e.,
that they're intended to be displayed to the user rather than
representing machine-readable codes). The example should reflect that
as well. I'd also remove the ambiguity about whether the field will
be present when rc == 200. It's simpler to require that it always be
present. Maybe:

"errors"
This will contain an array of strings indicating any errors that
occurred when making this request. These messages can be displayed to
the user to provide detail about the cause of the error. For example,
if a 500 error occurred in the request: [ "Internal Server Error" ].
If rc is 200, this field MUST be a zero length array. If rc is not
200, this field MUST contain at least one element.

> "data"
>     This will contain a different type of data depending on the type
> of request that was made, and is only present if rc is 200. This is
> controlled by the parameter gadgets.io.RequestParameters.CONTENT_TYPE,
> with values enumerated in gadgets.io.ContentType.

Should define the type of this field for each ContentType. This is
already specified somewhat in the docs for ContentType, but I think
more detail about how each type is interpreted would be good,
especially for ContentType.FEED.

Ideally, it would be nice if someone familiar with the specifics of an
implementation of this could provide this. If not, I can look at what
Shindig does and try to write something up.

> "oauthError"
>     If this value is specified, it indicates an OAuth-related error
> occurred. The value will be one of a set of string constants that can
> be used for programmatically detecting errors. The constants are
> undefined for opensocial-0.8, but implementers should attempt to agree
> on a set of useful constant values for standardization in
> opensocial-0.9.

That ship has sailed :-) 1.0 seems like a good time to actually
standardize this.

Using OAuth Problem Reporting extension seems like a good way to go:
http://oauth.pbworks.com/ProblemReporting

I believe this is what Shindig does in practice.

> "oauthErrorText"
>     If this value is specified, it indicates an OAuth-related error
> occurred. The value is free-form text that can be used to provide
> debugging information for gadget developers.

Is this text intended to be displayed to end-users as well?

Cheers,
-- Tim

[Jon]

For the "errors" field - is anyone counting on the first part of the
string to be the error code? The OS article shows it that way. I would
vote to remove it, and make the field a simple string for human
consumption, with the error code in rc, just wonder if that might
break any existing gadgets? Shindig is inconsistent in its usage, I
have been working on io.js to clean it up, but have hit some issues
with the unit tests and am debugging them now.

When the spec comes up for editing, I can write the part for TEXT,
JSON, and DOM, which we use. We don't use RSS so I could use some help
for that, or simply reverse engineer it.

Also we don't use OAuth, so any help there would be appreciated.