bug when returning wrong result from batch loader function

[uwemaurer]

It would be good if the cachified library would check the return value of the createBatch loader function before working with it.

The idea is to pass an array as input and get back an array of the same length with the corresponding results.

If the user makes a mistake by returning a different array length than the input array the cachified library does not detect this and goes into a bad state ("unsettled top-level await").

(It can easily happen when working with APIs which leave out missing data, eg load posts with id 1,2,3. but 2 was deleted so it returns an array with data for 1,3.)

See this example program:

import { cachified, createBatch } from "@epic-web/cachified";

const cache = new Map();

function getFromApi(ids: number[]) {
  return ids
    .filter((id) => id % 2 === 0)
    .map((id) => {
      return { id, data: `data:${id}` };
    });
}

async function getFreshValuesCorrect(idsThatAreNotInCache: number[]) {
  const result = getFromApi(idsThatAreNotInCache);
  const resultMap = new Map(result.map(item => [item.id, item]));  
  return idsThatAreNotInCache.map(id => resultMap.get(id));
}


async function getFreshValuesBad(idsThatAreNotInCache: number[]) {    
    return getFromApi(idsThatAreNotInCache);
}

function getUsersWithId(ids: number[], useBadImplementation: boolean) {
  const batch = createBatch(useBadImplementation ? getFreshValuesBad : getFreshValuesCorrect);

  return Promise.all(
    ids.map((id) =>
      cachified({
        getFreshValue: batch.add(id),

        cache,
        key: `entry-${id}`,
        ttl: 60_000,
      }),
    ),
  );
}

console.log(await getUsersWithId([1, 2, 3, 4, 5], false));

console.log(await getUsersWithId([11, 12, 13, 14, 15], true));

[uwemaurer]

I suspect that the problem is that, if the returned array is shorter, then there are some values at the end which never finish loading.

I think the fix would be to check the returned array length and throw an exception if not the same length and fail loading of all the provided inputs then

(because the partially provided results are likely wrong too because the mapping is not correct when there are missing elements in between)

[Xiphe]

Yeah, this seems absolutely reasonable! Would appreciate PRs to this.