Partial Errors caching

[mostafa-rz]

When I use ErrorPool in my resolver and add the cache directive to the field, it works as expected by the first request. I will get the result + error. For the second request, the result is loaded from the cache, therefore the error is not shown anymore in the final result.
I think the result should not be cached when an error exists, or at least it should be optional in the lighthouse config to respect the ErroPool.

[spawnia]

I see your point. Very roughly, I could see something the implementation looking something like this:

$errorsListener = $this->errorPool->listen();            

$resolved = $resolver($root, $args, $context, $resolveInfo);

$storeInCache = $maxAge
    ? static fn ($result): bool => $cache->put($cacheKey, ['data' => $result, 'errors' => $errorsListener->errors()], Carbon::now()->addSeconds($maxAge))
    : static fn ($result): bool => $cache->forever($cacheKey, ['data' => $result, 'errors' => $errorsListener->errors()]);

Resolved::handle($resolved, $storeInCache);

Would you be willing to implement this?

[mostafa-rz]

I am not sure about the idea. Because we put the resolver result into the cache(eloquent collection, etc.). In the next steps we make ExecutionResult object and there we add the errors. If there is an error we should not cache it at all.

[spawnia]

Ah, I misunderstood your intent then. I thought you wanted to remember the errors that occured in the case.

If you do not want to cache a result at all when any error happened, why not just throw instead of using ErrorPool?

[mostafa-rz]

no, you understand it correctly. What i mean is that in this part of the code:

$storeInCache = $maxAge
    ? static fn ($result): bool => $cache->put($cacheKey, ['data' => $result, 'errors' => $errorsListener->errors()], Carbon::now()->addSeconds($maxAge))
    : static fn ($result): bool => $cache->forever($cacheKey, ['data' => $result, 'errors' => $errorsListener->errors()]);

The $result can not contain 'errors' => $errorsListener->errors()'. It should be the pure result.

[mostafa-rz]

In testStoreResolverResultInCacheTest() test replace resolver mock and it is fails:

$this->mockResolver([
            'data' =>[
                'id' => 1,
                'name' => 'foobar',]
        ]);

[spawnia]

Yeah, we would need to unwrap the result and re-add the errors to the pool when retrieving the cached value.