starred/qBittorrent

Fork 0

mirror of https://github.com/qbittorrent/qBittorrent.git synced 2026-03-02 22:57:32 -05:00

Web API error handling + standard response #8914

New issue

Open

opened 2026-02-21 19:53:07 -05:00 by deekerman · 11 comments

deekerman commented

2026-02-21 19:53:07 -05:00

Owner

Originally created by @Piccirello on GitHub (Jul 22, 2019).

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.

Originally created by @Piccirello on GitHub (Jul 22, 2019). 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.

deekerman added the

Discussion

WebAPI

labels

2026-02-21 19:53:07 -05:00

deekerman commented

2026-02-21 19:53:09 -05:00

Author

Owner

@Piccirello commented on GitHub (Jul 22, 2019):

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"
    }
  ]

@Piccirello commented on GitHub (Jul 22, 2019): This is my proposal for the api response. It's intended to be simple and concise. API Response ```typescript { 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: ```typescript { success: 1, failed: 1, responses: [ { success: true, }, { success: false, error: "url is invalid" } ] ```

deekerman commented

2026-02-21 19:53:11 -05:00

Author

Owner

@glassez commented on GitHub (Jul 22, 2019):

This is my proposal for the api response.

Sorry, it has a little meaning without detailed description.

@glassez commented on GitHub (Jul 22, 2019): >This is my proposal for the api response. Sorry, it has a little meaning without detailed description.

deekerman commented

2026-02-21 19:53:11 -05:00

Author

Owner

@Piccirello commented on GitHub (Jul 22, 2019):

I've added more info to the proposal.

@Piccirello commented on GitHub (Jul 22, 2019): I've added more info to the proposal.

deekerman commented

2026-02-21 19:53:11 -05:00

Author

Owner

@Piccirello commented on GitHub (Aug 19, 2019):

@glassez do you have any thoughts on this proposal?

@Piccirello commented on GitHub (Aug 19, 2019): @glassez do you have any thoughts on this proposal?

deekerman commented

2026-02-21 19:53:11 -05:00

Author

Owner

@glassez commented on GitHub (Aug 21, 2019):

@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 commented on GitHub (Aug 21, 2019): >@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.

deekerman commented

2026-02-21 19:53:11 -05:00

Author

Owner

@glassez commented on GitHub (Aug 31, 2019):

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
  }
}

@glassez commented on GitHub (Aug 31, 2019): This is my proposal for the API response. IMO, it's simpler and more flexible than @Piccirello's proposal. Single-job response: ```typescript { // 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: ```typescript { // 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 } } ```

deekerman commented

2026-02-21 19:53:11 -05:00

Author

Owner

@Piccirello commented on GitHub (Apr 25, 2020):

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
    },

@Piccirello commented on GitHub (Apr 25, 2020): 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: ```diff { // 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 }, ```

deekerman commented

2026-02-21 19:53:11 -05:00

Author

Owner

@glassez commented on GitHub (Apr 26, 2020):

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).

@glassez commented on GitHub (Apr 26, 2020): >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).

deekerman commented

2026-02-21 19:53:11 -05:00

Author

Owner

@Piccirello commented on GitHub (Apr 26, 2020):

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.

@Piccirello commented on GitHub (Apr 26, 2020): > 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.

deekerman commented

2026-02-21 19:53:11 -05:00

Author

Owner

@glassez commented on GitHub (Apr 27, 2020):

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 commented on GitHub (Apr 27, 2020): >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".

deekerman commented

2026-02-21 19:53:11 -05:00

Author

Owner

@glassez commented on GitHub (Apr 27, 2020):

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.

@glassez commented on GitHub (Apr 27, 2020): >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.