RFC: improvements to rest#request

[SimonWoolf]

The design of rest#request (with the HttpPaginatedResource), introduced in 2016 in ably/docs#133 , is quite weird.

Having error messages primarily exposed as res.errorMessage & res.errorCode is inconsistent with how every other method works, isn't a good fit for what we're doing these days with putting more information into the errorinfo (such as help urls), and doesn't expose the batchResponse (for partial success) in a nice way. The batchResponse is currently exposed just by the fact that the whole response body ({error: ErrorInfo, batchResponse: Array<...>}) ends up as the first element of the items array, which is a pretty weird and unintuitive api.

We'll likely be using this more and more as batch publishing goes into GA, token revocation uses it, etc. (All those will probably end up with first class support in all client libs eventually, but seems unlikely that'll be a high priority any time soon, so having a good rest#request seems important)

Couple sketches of possible alternative APIs:

1. rest#request signals errors the same way we do in every other api call (eg in js, the err argument of the callback, or rejected promise).
   For the 'partial success' case, ErrorInfos will have an option batchResponse property (Array<JsonObject>), which is populated with the batchResponse array.

2. rest#request signals errors the same way we do in every other api call (eg in js, the err argument of the callback, or rejected promise).
   For the 'partial success' case, we change the realtime implementation (triggered by a version flag of some kind) to just return the batchresponse array as the top level response, which would then always be Array<JSONObject>, e.g. Array<{channel: string, messageId?: string, error?: ErrorInfo}> for batch publish. So we deprecate the "Batched response includes errors" errorinfo.
   The http request as a whole nominally suceeds (ie http status code is 201), and if someone wants to see if every individual item succeeded, they can check batchResponses.every(br => !br.error), or the specific language equivalent.

TBD: how we deal with the breaking change that this would introduce, given that it seems unlikely we're going to be releasing a 2.0 spec any time soon.

Opinions on those two, further suggestions of better request APIs, or arguments for keeping the original version all appreciated.

@QuintinWillison @AndyNicks @paddybyers @mattheworiordan @ben-pickering @lmars

┆Issue is synchronized with this Jira Task by Unito

[paddybyers]

The problem with the suggested alternatives is that they require the called API to conform to certain conventions. The point of request() is to enable a library to use some as-yet unspecified future API; the current API design was based on the idea what we don't know what those future APIs will be, and therefore we shouldn't be assuming that they will conform to the conventions we have today.

I do agree that this leads to an API that's really awkward to use.

Should we specify that something like the existing API is still available (maybe httpRequest() or something, reinforcing the idea that it really is just fully exposing an HTTP primitive), and then have some more idiomatic wrappers on top?

[lmars]

I agree that rest#request returning a response with errorCode and errorMessage fields should be replaced with rest#request returning an ErrorInfo when the request is not successful, and it seems the sticking point here is how that affects things that can partially succeed, but perhaps we introduce rest#batch_request which returns a paginated response of (item, error) pairs when it fully or partially succeeds, and document that rest#request is not suitable for batched responses?

We could mandate that rest#batch_request is added either before or at the same time as the semantics of rest#request are changed, and the CHANGELOG would point those who were using rest#request for batch publishing to now use rest#batch_request?

[paddybyers]

We could mandate that rest#batch_request is added either before or at the same time as the semantics of rest#request are changed

but then that further violates the principle:

the idea what we don't know what those future APIs will be, and therefore we shouldn't be assuming that they will conform to the conventions we have today

[SimonWoolf]

If we change the rest api so drastically that we break the {error: ErrorInfo} convention then that's going to need client lib rewrites anyway, so not much point planning for that in advance, surely.

(my second proposal doesn't involve the client lib making any assumptions about a batchresponse, since it just returns that as the regular array response body and gets rid of the nested batch-response-includes-errors error response)

But, ok, if we did really want to leave rest#request with its current semantics (which does have the advantage of avoiding having to figure out how we do backcompat), then per Lewis's suggestion we can add a new rest#requestBatch with saner semantics (but which makes stronger assumptions about realtime conventions), sure. rest#request would continue to exist, but at least we'd have requestBatch that we can recommend for use with bulk publish, bulk presence get, token revocation, etc.

[lmars]

How about:

• rest#http_request is a lower level version of the current rest#request which returns an HTTP response (e.g. as a (status, headers, body) thing, no further interpretation)
• rest#request is the method for sending a single "Ably request", either returning a successful response (status 200-299), or an error (ErrorInfo)
• rest#batch_request is the method for sending multiple "Ably requests", either returning a fully or partially successful response (at least one inner request succeeded), or an error (ErrorInfo)

So we are changing the semantics of rest#request to be confined to "single Ably request" APIs, but as Paddy suggested pushing the generic method down a level to support future APIs that may not fit either the single or batch request API.

[SimonWoolf]

So we are changing the semantics of rest#request to be confined to "single Ably request" APIs,

That would be a sufficiently breaking change that we couldn't really do it except with a client lib major version change (since people use rest#request now for things like bulk publishing). And.. yeah, we'll need to figure out how to do that sooner or later. But realistically it would be towards the 'later' end.

Maybe introduce rest#http_request, rest#request_batch, and rest#request_single now, leave rest#request and mark as deprecated pending the next major version?

Also though.. the only reason for having request_batch and request_single as separate methods is to accommodate languages that insist that a JSONObject-equivalent can't be an array, so it needs to know whether to expect an array or object response, right? But are there actually any of those? ably-java is usually the client lib that's the least flexible type-system-wise and it uses JsonElement which can be either a JsonObject or JsonArray. Is this single/batch split actually a distinction worth carrying forward?

(Is the argument 'you need to know whether it's an object or array to know how to do something with it'? If so I'm sceptical. If you want to interact with it programmatically you really need to know the detailed response type for that api call, which you'll look up in the documentation. I don't see why having the method assure you that "it's an array of something" is really any more useful than knowing 'it's something, which might be an array of other things').

[QuintinWillison]

Hi 👋 - sorry that I've not got around to engaging on this one yet. I'm planning to look at it next week.

[QuintinWillison]

Loosely related to #32

[SimonWoolf]

@QuintinWillison gently bumping this up your todo list

[QuintinWillison]

Sorry, this is still on my (long) list of things to look at. I want to take a closer look at how this has been implemented across a few client libraries in practice, how that differs (or not) from other APIs in that library and to draw some diagrams to ensure my understanding is complete. I also want to investigate whether the spec has too much overreach in each case, in respect of how it mandates the shape of client library APIs (Ably vs language/platform-idiomatic conventions).

[SimonWoolf]

I think we should consider this for 2.0 -- we've already committed to make one change to the rest#request api (adding a version) in 2.0, so if we're going to make other changes to that api, it would make sense to do them at the same time, to minimise the number of customer disruptions.

[sync-by-unito]

➤ Automation for Jira commented:

The link to the corresponding Jira issue is https://ably.atlassian.net/browse/SDK-2844

[SimonWoolf]

NB: we've already done the 'we can change the realtime response...' part of option 2 as part of the new batch response #139 . So option 2 is looking pretty good