Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Failed resolvers should not be cached #115

Open
wilkerlucio opened this issue Dec 10, 2021 · 3 comments
Open

Failed resolvers should not be cached #115

wilkerlucio opened this issue Dec 10, 2021 · 3 comments
Assignees
Labels
bug Something isn't working design phase

Comments

@wilkerlucio
Copy link
Owner

When a resolver throws an error, it must not cache that error value.

As of now, it does cache that error value, and in case it's a persistent cache you end up always getting the error.

@wilkerlucio wilkerlucio added the bug Something isn't working label Dec 10, 2021
@wilkerlucio
Copy link
Owner Author

At the same time, inside the same request its a good performance characteristic to avoid re-calling a resolver that threw an error with that same input.

This needs more consideration.

@andrewzhurov
Copy link

andrewzhurov commented Sep 3, 2022

It seems that we would be save to cache only pure functions, and caching errors would be desirable as well (we'd always get the same errors if we run it the same).
Whereas caching of impure functions seems like a risky idea both for errors and results (!) (as these can change, depending on the outside's state).
So it seems to be a good default behaviour to cache only pure functions.

How would we know if a function is pure?
There are no easy ways known to me..
Perhaps we could try to infer it from function's body (akin to how it's done with inference of inputs and outputs) or run it in an isolated environment and see if it behaves the same, but these may not be accurate.
A good way seems to be to let users mark functions as pure/impure. (impure being the default (safe defaults))
And perhaps cacheable/not cacheable. (In case some users would like to cache an impure function at their own risk) (not cacheable being the default) Upd: I found that this mechanism is already implemented, as described in the docs. However, default is to cache, which is risky for impure functions.

@wilkerlucio
Copy link
Owner Author

I'm thinking that this might be better as a configurable option per resolver, we can keep it as-is for the default and allow the user to flag a resolver to set the behavior. This way, the user will have more control over handling exceptions per resolver.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working design phase
Projects
None yet
Development

No branches or pull requests

2 participants