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

redis cache issues #1130

Closed
jakeblaxon opened this issue Nov 6, 2020 · 7 comments
Closed

redis cache issues #1130

jakeblaxon opened this issue Nov 6, 2020 · 7 comments
Labels
bug Something isn't working

Comments

@jakeblaxon
Copy link

jakeblaxon commented Nov 6, 2020

Hello, I've noticed two issues when trying to use the redis-cache plugin:

  1. The cache's get and set commands don't parse and stringify the values correctly. For example, we pass an object instead of a string as the value to redis's set command, which causes it to error out.
  2. The cache doesn't handle aliases. When using aliases in my queries, the response comes back correctly the first time (when it gets cached) but then doesn't map the aliases correctly on subsequent pulls. These fields instead get mapped to null.

Let me know if you have any questions. Thanks!

@ardatan ardatan added the bug Something isn't working label Nov 9, 2020
@ardatan
Copy link
Owner

ardatan commented Nov 9, 2020

Could you try with this canary version?
#1134 (comment)

@jakeblaxon
Copy link
Author

That looks to have solved the first issue. I'm still having problems with aliases, though.

@jakeblaxon
Copy link
Author

jakeblaxon commented Nov 9, 2020

Looking at the difference, I see that the following fields are in the initial result but not the cached result:

    [Symbol(subschemaErrors)]: [],
    [Symbol(initialSubschema)]: {
      schema: [GraphQLSchema],
      executor: [Function: executor],
      transforms: [Array]
    }

Since symbols can't be stringified, this makes sense that they're getting left out. I wonder if the alias information is somehow being stored in these fields that are getting discarded. I am using a custom transform that uses @graphql-tools/stitch to transform the schema, so maybe that has something to do with it.

@jaggad
Copy link
Contributor

jaggad commented Jun 9, 2021

@jakeblaxon We're having this alias issue also, did you find any workaround or solution to this bug?

@jakeblaxon
Copy link
Author

@jackedgson I ended up using the apollo-server-cache-redis and apollo-server-plugin-response-cache packages instead, since I'm using apolloserver to host the schema that graphql-mesh generates.

One note: I'm using apollo-server-cache-redis 1.2.2, which required a patch to work correctly. I used patch-package to apply the following patch upon npm install:
filename: apollo-server-cache-redis+1.2.2.patch

diff --git a/node_modules/apollo-server-cache-redis/dist/RedisCache.js b/node_modules/apollo-server-cache-redis/dist/RedisCache.js
index 91c4295..3827f78 100644
--- a/node_modules/apollo-server-cache-redis/dist/RedisCache.js
+++ b/node_modules/apollo-server-cache-redis/dist/RedisCache.js
@@ -27,6 +27,7 @@ class RedisCache {
         });
     }
     set(key, value, options) {
+        if (!this.isReady()) return Promise.resolve(undefined);
         return __awaiter(this, void 0, void 0, function* () {
             const { ttl } = Object.assign({}, this.defaultSetOptions, options);
             if (typeof ttl === 'number') {
@@ -38,6 +39,7 @@ class RedisCache {
         });
     }
     get(key) {
+        if (!this.isReady()) return Promise.resolve(undefined);
         return __awaiter(this, void 0, void 0, function* () {
             const reply = yield this.loader.load(key);
             if (reply !== null) {
@@ -47,6 +49,7 @@ class RedisCache {
         });
     }
     delete(key) {
+        if (!this.isReady()) return Promise.resolve(undefined);
         return __awaiter(this, void 0, void 0, function* () {
             return yield this.client.del(key);
         });
@@ -62,6 +65,10 @@ class RedisCache {
             return;
         });
     }
+    isReady() {
+        if (!this.client || !this.client.status) return false;
+        return this.client.status.toLowerCase() === 'ready';
+    }
 }
 exports.RedisCache = RedisCache;
 //# sourceMappingURL=RedisCache.js.map

This was from a while ago, though, so they may have fixed it by now.

@ardatan
Copy link
Owner

ardatan commented Jun 9, 2021

Did you try the canary version I mentioned here?
#2292 (comment)

@theguild-bot theguild-bot mentioned this issue Aug 11, 2022
@ardatan
Copy link
Owner

ardatan commented Mar 31, 2023

Closing the issue due to the inactivity. Feel free to create a new one if the issue persists.

@ardatan ardatan closed this as completed Mar 31, 2023
@theguild-bot theguild-bot mentioned this issue Sep 28, 2023
This was referenced Apr 30, 2024
This was referenced May 7, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

3 participants