Web API error handling + standard response

[Piccirello]

The Web API should provide more granular results and well defined behavior when executing a request and handling errors. This issue is to discuss the format for the standard Web API response.

[Piccirello]

This is my proposal for the api response. It's intended to be simple and concise.

API Response

{
  success: number,  // the number of requests that succeeded,
  failed: number,   // the number of requests that failed,
  responses: [  // array of results in the order in which the parameters were received
    {
      success: bool,
      error: String || undefined,
      data: any || undefined
    },
    ...
  ]
}

For example, imagine a POST request to /api/v2/torrents/addTrackers with a valid hash and two urls. The first hash is valid and the second is invalid. The response would look like this:

{
  success: 1,
  failed: 1,
  responses: [
    {
      success: true,
    },
    {
      success: false,
      error: "url is invalid"
    }
  ]

[glassez]

This is my proposal for the api response.

Sorry, it has a little meaning without detailed description.

[Piccirello]

I've added more info to the proposal.

[Piccirello]

@glassez do you have any thoughts on this proposal?

[glassez]

@glassez do you have any thoughts on this proposal?

Yes. I just don't have time to do it all at once, so I'm commenting on the higher priority issues/PRs first. In addition, I'm not very comfortable writing code samples from my smartphone, so I'll do it as soon as I'm near my computer in my free time.

[glassez]

This is my proposal for the API response. IMO, it's simpler and more flexible than @Piccirello's proposal.

Single-job response:

{
  // Status of entire request processing.
  // Can be assigned to one of global 
  // or particular action related status codes
  status: number,
  // Message that describes the status
  message: string,
  // Result of request processing
  result: {
    // particular action defined fields
  }
}

Multijob response:

{
  // Status of entire request processing.
  // Can be assigned to one of global status codes
  status: number,
  // Message that describes the status
  message: string,
  // Result of request processing
  result: {
    // set of single-job responses mapped by job ID
  }
}

[Piccirello]

I'm in favor of the global status codes you proposed. I would still like to see a top-level count of the number of items that failed/succeeded. Mapping multi-job responses by ids does make more sense than array indexing.

Multijob response:

  {
   // Status of entire request processing.
   // Can be assigned to one of global status codes
   status: number,
   // Message that describes the status
   message: string,
+  // Number of tasks that succeeded
+  success: number,
+  // Number of tasks that failed
+  failed: number,
   // Result of request processing
   result: {
     // set of single-job responses mapped by job ID
   }
 }

I also think we should be more prescriptive about the action-defined fields. Copied from above:

    {
      success: bool,
      error: String || undefined,
      data: any || undefined
    },

[glassez]

Mapping multi-job responses by ids does make more sense than array indexing.

What sense do you find in it? After all, you still have to process them all. However, I won't mind too much if you want to complicate it. Maybe it's really useful for something.

I also think we should be more prescriptive about the action-defined fields.

I agree generally. But I would have "status: OK|Failed" instead of your "success" field. And "errorMessage" instead of "error" (it should be optional and exist only in case of "status: Failed"). Or maybe pass error message in "data" field instead?

I would still like to see a top-level count of the number of items that failed/succeeded.

This looks like a loophole for ambiguity... it may not match the actual values from the subtask results. But of course, this can be avoided by implementing automatic filling in of these fields in API controllers. We can provide addResult(int status, QJsonValue data) that accumulates results and calculate total numbers of succeeded/failed ones.

The only thing I don't like is that each Action will be responsible for maintaining consistency of its result (i.e. if it is "multijob" action, it should always return "multijob" result, and vice versa). Shouldn't we completely unify this? So that all Actions return the Result in the same form (as "multijob" one, but "singlejob" Actions will always have only one element in it).

[Piccirello]

What sense do you find in it? After all, you still have to process them all.

I was agreeing with you! My original proposal had responses as an array, but you suggested instead using a result map that's mapped by job id, which I can get behind.

I agree generally. But I would have "status: OK|Failed" instead of your "success" field. And "errorMessage" instead of "error" (it should be optional and exist only in case of "status: Failed"). Or maybe pass error message in "data" field instead?

Agreed on using error instead of errorMessage, though I'd keep it outside of data. It lends itself well to a general rule: if successful, use the data field, otherwise use error.

If Ok and Failed are they only two possible status values than a boolean still seems more straightforward to me.

This looks like a loophole for ambiguity... it may not match the actual values from the subtask results. But of course, this can be avoided by implementing automatic filling in of these fields in API controllers. We can provide addResult(int status, QJsonValue data) that accumulates results and calculate total numbers of succeeded/failed ones.

+1 on this method.

Shouldn't we completely unify this? So that all Actions return the Result in the same form (as "multijob" one, but "singlejob" Actions will always have only one element in it).

I was pondering this as well. While I prefer single-job actions to return a single response (no array), from an implementation standpoint it is definitely less error prone to treat every response as multi-job. I'm fine with either.

[glassez]

If Ok and Failed are they only two possible status values than a boolean still seems more straightforward to me.

No. I suppose that there should be several error statuses to indicate different kind of errors that can be handled differently at the client side. As for error field. I still believe that returning error description strings from API is generally bad idea. IMO, it's good point to differentiate the data and its representation (in case of error too). So I would return error status/code and let client application to represent it. But since statuses/codes alone are not enough to provide all the necessary information about some errors (for example, file names, etc.), we could return in error field something like an "exception object" that stores all the necessary data. Then it will be enough for us to have a boolean type for the status field, and the specific error code will be in the "exception object".

[glassez]

While I prefer single-job actions to return a single response (no array)

👍 If we can find a convenient and reliable way to declare various types of Actions (multijob and singlejob), I will prefer it.