Review RTKQ "reject on existing query" behavior

[markerikson]

RTKQ uses createAsyncThunk inside, and I think we use condition to check if a query is already in progress and we should bail out. Unfortunately, that causes a rejected action to be dispatched with the reason.

I've always found this behavior to be sketchy, and I wonder if it may be one of the reasons for "slowness" that have been mentioned a few times when a query hook gets rendered in a list. If, say, 100 components mount, and the first one starts the request, the other 99 are likely to dispatch rejected actions.

I think we ought to reconsider doing that. If necessary, we ought to think about modifying createAsyncThunk to allow skipping the rejected action if condition returns false.

We probably can't do this for 1.9, but maybe for 2.0?

/cc @phryneas .

[phryneas]

Debounce-Batching those would make more sense, maybe with a few milliseconds of a window.

Other places where batching makes sense: https://github.com/reduxjs/redux-toolkit/blob/67d41a6f8c29603023f586a24689817cd66d2917/packages/toolkit/src/query/core/buildMiddleware/windowEventHandling.ts#L37 (dispatches removeQueryResult or refetchQuery which will then lead to a pending or rejected action again)

https://github.com/reduxjs/redux-toolkit/blob/20fd4d8f2abdb662af7ecc9de5fd382ae5bd3719/packages/toolkit/src/query/core/buildMiddleware/invalidationByTags.ts#L69 (same)

[phryneas]

So essentially, all pending, rejected (with condition) and removeQueryResult actions should be batched.

[markerikson]

Copying notes from Discord:

[5:36 PM] phryneas : cAT can already skip the rejected action, in fact that's the default behaviour. We use the rejection for something I can't remember. [5:38 PM] phryneas :

          queryThunk.rejected,
          (draft, { meta: { condition, arg, requestId }, error, payload }) => {
            // request was aborted due to condition (another query already running)
            if (condition && arg.subscribe) {
              const substate = (draft[arg.queryCacheKey] ??= {})
              substate[requestId] =
                arg.subscriptionOptions ?? substate[requestId] ?? {}
            }
          }

[5:38 PM] phryneas : the rejection will occur instead of a pending so we use it to start tracking the component subscription [5:39 PM] acemarke : hmm. drat, we do track subscriptions in state [5:39 PM] acemarke : wonder if there's any way to batch that.. [5:41 PM] phryneas : if we add a batch functionality to RTK by default, sure. That could just debounce-batch everything with an action.meta.[debouncableSymbol] or something like that - that would make it 100% compatible with existing code [5:42 PM] acemarke : could maybe do it as part of the existing middleware? [5:42 PM] phryneas : can middleware debounce? i thought that needs a store enhancer [5:43 PM] acemarke : a middleware can do anything to any action :) [5:43 PM] acemarke : modify, log, intercept, delay, whatever [5:43 PM] phryneas : sure, but I'm not sure if that will impact store.subscribe [5:43 PM] acemarke : subscribe only runs when an action reaches the real store [5:43 PM] phryneas : then we could add something like that, yeah [5:44 PM] acemarke : so if we added logic that says "look for anything that resembles a 'rejected due to already running query' action", gather those up, and dispatch one batched action on a short delay... [5:44 PM] phryneas : or really just add some kind of identifier to the meta. There are more things in here we could batch

[markerikson]

Done with 1.9.

[nouman91]

@markerikson This is still happening for me in version 1.9.5. Check the below console output for my error middleware. Any thoughts and how should I prevent this?

{
    "type": "api/executeQuery/rejected",
    "payload": {
        "status": 422,
        "data": {
            "error_code": 10202,
            "error_message": "Invalid ",
            "module_name": "**"
        }
    },
    "meta": {
        "baseQueryMeta": {
            "request": {},
            "response": {}
        },
        "RTK_autoBatch": true,
        "arg": {
            "type": "query",
            "subscribe": true,
            "subscriptionOptions": {
                "pollingInterval": 0
            },
            "endpointName": "**",
            "originalArgs": [
                "14bfa34d-e087-4147-b73a-18417e290e1b",
                "a35184c6-5ed0-4c95-a20e-93bba7024de3"
            ],
            "queryCacheKey": "**([\"14bfa34d-e087-4147-b73a-18417e290e1b\",\"a35184c6-5ed0-4c95-a20e-93bba7024de3\"])"
        },
        "requestId": "9E6Tp-e6iIQKShfmLp0Ye",
        "rejectedWithValue": true,
        "requestStatus": "rejected",
        "aborted": false,
        "condition": false
    },
    "error": {
        "message": "Rejected"
    }
}

[EskiMojo14]

No, that's completely unrelated to this issue.

This issue is talking about condition rejection actions, which would have condition: true in the meta. See dispatchConditionRejection.

Your error is an actual error being returned from your base query, indicating your request failed.

[nouman91]

Ok but where does this come from & why?

"error": {
        "message": "Rejected"
    }

[EskiMojo14]

That's what action.error gets set to when you use rejectWithValue inside of an async thunk, because the actual error is action.payload, as you can see.

[nouman91]

Thank you @EskiMojo14 for being so responsive and helpful, I appreciate it.

In my use case, I want to get all the API errors in error middleware and display them to the user. So far I have been encountering a few problems regarding accessing the error message(as discussed in this thread: https://github.com/reduxjs/redux-toolkit/issues/3618). A few of the APIs have error messages in the error. message field but few have only 'rejected' in error. message field. I am not sure why is that and which field should I be using to access the error message.

[EskiMojo14]

please take it back to that thread - as I said, it's entirely unrelated to this issue 😉

[nouman91]

Done!