frontend: fix repeatStreamFunc for list requests

[msuret]

errCb parameter is the 3rd argument of get function returned by singleApiFactory, whereas it's only the 2nd argument of list. This causes repeatStreamFunc to misbehave for list calls because the error callback index is hard-coded to 2.
In my case I couldn't list custom resources with deprecated API versions.

[joaquimrocha]

Thanks @msuret . This whole file is a hack, and we only recently added tests enough to be confident in changing it.
I think a more proper way of fixing this issue is to just pass the list arguments as is, to the functions that repeatStreamFunc calls. The only reason why we special case the errCb is because we need to use it in repeatStreamFunc, so we should have this arg separate, even if repeated in the list of args. This way we don't have to keep guessing indexes for function calls. Not sure if my idea is clear. WDYT @msuret ?
@skoeva , can you add a test for these functions?

[msuret]

@joaquimrocha I am not sure to understand what you mean by "have this arg separate". Is it something like repeatStreamFunc always appending the modified error callback at the end of the argument list, then list and get would accept a new errCbOverride argument that would take precedence over errCb when present?

Other solutions I can see (But they require more changes and thus are more risky):

• move errCb to always be the first or last argument, so it could be replaced without guessing its index.
• have a single options argument with an errCb attribute which would be easy to replace.

[joaquimrocha]

What I meant was simpler:

async function repeatStreamFunc(
  apiEndpoints: ApiFactoryReturn[],
  funcName: keyof ApiFactoryReturn,
  errCb: StreamErrCb,
  ...args: any[]
)

Since the problem here is that we are passing the errCb as an arg to the repeatStreamFunc and then trying to fit it into the args with which we call funcName, we should treat the errCb independently from whether it is in args.
So we could do:

async function repeatStreamFunc(
  apiEndpoints: ApiFactoryReturn[],
  errCb: StreamErrCb,
  funcName: keyof ApiFactoryReturn,
  ...args: any[]
)

And inside the repeatStreamFunc, we call funcName with its args only, without trying to fit the errCb in.
So all is left to do is to make sure we call repeatStreamFunc with the args part being passed with the exact args that list/get are using. This way, errCb will be passed twice, first directly as an arg for repeatStreamFunc, and second as part of the args, but we never have to compose the arg list again from within repeatStreamFunc.

Since repeatStreamFunc is internal, it's totally fine to do this IMO.

WDYT?

[skoeva]

+1 to the earlier comment, this file is due for a refactor and the test suite was only added recently. I agree with treating errCb independently as a potential approach, this avoids the guess work in getting its index. re: testing for repeatStreamFunc, wouldn't we need to expose it? @joaquimrocha

[msuret]

@joaquimrocha I'm not sure that would work, from my understanding repeatStreamFunc doesn't just pass errCb as it is to funcName but wraps it in order to try a different endpoint in case of 404.

[joaquimrocha]

@joaquimrocha I'm not sure that would work, from my understanding repeatStreamFunc doesn't just pass errCb as it is to funcName but wraps it in order to try a different endpoint in case of 404.

Ah! You are 100% correct. I had overlooked that part. Sorry.
In any case, our intention is to make those list/get/etc functions get params as an object, and we also want to check with the API server what version of the API/group we have, so we can make more informed fallbacks.

I will merge your PR as a fix for what we have, till we have a proper fix. Thank you!