diff --git a/src/v2/Artsy/Relay/createRelaySSREnvironment.ts b/src/v2/Artsy/Relay/createRelaySSREnvironment.ts index 0b1e2c52a20..f23e98e04ad 100644 --- a/src/v2/Artsy/Relay/createRelaySSREnvironment.ts +++ b/src/v2/Artsy/Relay/createRelaySSREnvironment.ts @@ -9,7 +9,6 @@ import { data as sd } from "sharify" import { RelayNetworkLayer, - // cacheMiddleware, errorMiddleware, loggerMiddleware, urlMiddleware, diff --git a/src/v2/Artsy/Relay/middleware/cache/Cache.ts b/src/v2/Artsy/Relay/middleware/cache/Cache.ts index fb499330e7a..9329bf21763 100644 --- a/src/v2/Artsy/Relay/middleware/cache/Cache.ts +++ b/src/v2/Artsy/Relay/middleware/cache/Cache.ts @@ -2,7 +2,6 @@ import { QueryResponseCache } from "relay-runtime" import createLogger from "v2/Utils/logger" import RelayQueryResponseCache from "relay-runtime/lib/network/RelayQueryResponseCache" import { isDevelopment, isServer } from "lib/environment" -import { once } from "lodash" const logger = createLogger("v2/Artsy/middleware/cache/Cache") @@ -14,7 +13,7 @@ export interface CacheConfig { export class Cache { cacheConfig: CacheConfig - enableServerSideCaching: boolean + enableServerSideCache: boolean relayCache: RelayQueryResponseCache redisCache: { @@ -27,7 +26,7 @@ export class Cache { constructor(cacheConfig: CacheConfig) { this.cacheConfig = cacheConfig - this.enableServerSideCaching = + this.enableServerSideCache = isServer && !this.cacheConfig.disableServerSideCache this.initRelayCache() this.initRedisCache() @@ -38,7 +37,7 @@ export class Cache { } initRedisCache() { - if (!this.enableServerSideCaching) { + if (!this.enableServerSideCache) { return } @@ -47,7 +46,7 @@ export class Cache { const client = redis.createClient({ url: isDevelopment ? null : process.env.OPENREDIS_URL, - retry_strategy: this.handleRedisRetry, + retry_strategy: this.handleRedisError, }) this.redisCache = { @@ -57,36 +56,31 @@ export class Cache { get: promisify(client.get).bind(client), set: promisify(client.set).bind(client), } - - client.on( - "error", - once(error => { - logger.error("REDIS_CONNECTION_ERROR", error) - this.enableServerSideCaching = false - }) - ) } - handleRedisRetry(retryOptions) { - if (retryOptions.error && retryOptions.error.code === "ECONNREFUSED") { - return new Error("The server refused the connection") - } - if ( - retryOptions.total_retry_time > - process.env.PAGE_CACHE_RETRIEVAL_TIMEOUT_MS - ) { - return new Error( - `Retry time exhausted: ${process.env.PAGE_CACHE_RETRIEVAL_TIMEOUT_MS}ms` - ) + handleRedisError(retryOptions) { + const TIMEOUT = Number(process.env.PAGE_CACHE_RETRIEVAL_TIMEOUT_MS) + const MAX_RETRIES = 10 + + const logAndError = errorMsg => { + logger.error(errorMsg) + return new Error(errorMsg) } - // End reconnecting with built in error - if (retryOptions.attempt > 10) { - return undefined + switch (true) { + case retryOptions.error && retryOptions.error.code === "ECONNREFUSED": { + return logAndError("[Redis] The server refused the connection") + } + case retryOptions.total_retry_time > TIMEOUT: { + return logAndError(`[Redis] Retry time exhausted: ${TIMEOUT}ms`) + } + case retryOptions.attempt > MAX_RETRIES: { + return logAndError(`[Redis] Retry attempts exceeded: ${MAX_RETRIES}`) + } } - const reconnectAfter = Math.min(retryOptions.attempt * 100, 3000) - return reconnectAfter + const reconnectAfterTime = Math.min(retryOptions.attempt * 100, 3000) + return reconnectAfterTime } getCacheKey(queryId, variables) { @@ -98,7 +92,7 @@ export class Cache { let cachedRes = this.relayCache.get(queryId, variables) // No cache in relay store, check redis - if (this.enableServerSideCaching && !cachedRes) { + if (this.enableServerSideCache && !cachedRes) { const cacheKey = this.getCacheKey(queryId, variables) try { @@ -114,11 +108,11 @@ export class Cache { return cachedRes } - async set(queryId, variables, res, { cacheConfig }) { + async set(queryId, variables, res, options) { this.relayCache.set(queryId, variables, res) // Store in redis during server-side pass - if (this.enableServerSideCaching && !cacheConfig.force) { + if (this.enableServerSideCache && !options?.cacheConfig?.force) { const cacheKey = this.getCacheKey(queryId, variables) try { diff --git a/src/v2/Artsy/Relay/middleware/cache/__tests__/Cache.jest.ts b/src/v2/Artsy/Relay/middleware/cache/__tests__/Cache.jest.ts new file mode 100644 index 00000000000..254d6e7b6c4 --- /dev/null +++ b/src/v2/Artsy/Relay/middleware/cache/__tests__/Cache.jest.ts @@ -0,0 +1,182 @@ +/** + * @jest-environment node + */ + +import { Cache } from "../Cache" + +jest.mock("lib/environment") + +describe("Cache", () => { + const getCache = (props = {}) => { + return new Cache({ + size: 10, + ttl: 900000, + ...props, + }) + } + + afterEach(() => { + jest.restoreAllMocks() + }) + + describe("enableServerSideCaching", () => { + it("disables if disableServerSideCache=false", () => { + const cache = getCache({ disableServerSideCache: true }) + expect(cache.enableServerSideCache).toBe(false) + }) + + it("enables if disableServerSideCache=true", () => { + const cache = getCache({ disableServerSideCache: false }) + expect(cache.enableServerSideCache).toBe(true) + }) + }) + + it("initializes a relay cache", () => { + const spy = jest.spyOn(Cache.prototype, "initRelayCache") + const cache = getCache() + expect(spy).toHaveBeenCalled() + expect(cache.relayCache).toBeTruthy() + }) + + it("initializes a redis cache", () => { + const spy = jest.spyOn(Cache.prototype, "initRedisCache") + const cache = getCache() + expect(spy).toHaveBeenCalled() + expect(cache.redisCache).toBeTruthy() + }) + + describe("redis errors", () => { + it("throws error if ECONNREFUSED", () => { + const cache = getCache() + const err = cache.handleRedisError({ + error: { + code: "ECONNREFUSED", + }, + }) as Error + expect(err.message).toContain("The server refused the connection") + }) + + it("throws error if retrieval time exceeds PAGE_CACHE_RETRIEVAL_TIME", () => { + process.env.PAGE_CACHE_RETRIEVAL_TIMEOUT_MS = "10" + const cache = getCache() + const err = cache.handleRedisError({ + total_retry_time: 100000, + }) as Error + expect(err.message).toContain("Retry time exhausted") + }) + + it("throws error if number of attempts exceeds MAX_RETRIES", () => { + const cache = getCache() + const err = cache.handleRedisError({ + attempt: 11, + }) as Error + expect(err.message).toContain("Retry attempts exceeded") + }) + + it("returns a retry time that backs off", () => { + const cache = getCache() + ;[...new Array(10)].forEach(index => { + const retryTime = cache.handleRedisError({ + attempt: index, + }) as Error + expect(retryTime).toEqual(index * 100) + }) + }) + }) + + it("returns a cache key", () => { + const cache = getCache() + expect(cache.getCacheKey("ArtistQuery", { slug: "picasso" })).toEqual( + JSON.stringify({ + queryId: "ArtistQuery", + variables: { + slug: "picasso", + }, + }) + ) + }) + + describe("setting and getting cache", () => { + describe("client", () => { + it("sets / gets the cache by cacheKey", async () => { + const cache = getCache() + cache.enableServerSideCache = false + const queryId = "ArtistQuery" + const variables = { slug: "picasso" } + const response = "found response" + const options = { cacheConfig: { force: false } } + cache.set(queryId, variables, [response], options) + expect(await cache.get(queryId, variables)).toEqual( + expect.objectContaining({ 0: response }) + ) + }) + }) + + describe("server", () => { + it("does not set cache if enableServerSideCaching=false", async () => { + const cache = getCache() + cache.enableServerSideCache = false + cache.redisCache.get = jest.fn() + const queryId = "ArtistQuery" + const variables = { slug: "picasso" } + const response = "found response" + const options = { cacheConfig: { force: false } } + cache.set(queryId, variables, [response], options) + await cache.get(queryId, variables) + expect(cache.redisCache.get).not.toHaveBeenCalled() + }) + + it("does not set cache if cacheConfig.force=true", async () => { + const cache = getCache() + cache.enableServerSideCache = true + cache.redisCache.get = jest.fn() + const queryId = "ArtistQuery" + const variables = { slug: "picasso" } + const response = "found response" + const options = { cacheConfig: { force: true } } + cache.set(queryId, variables, [response], options) + await cache.get(queryId, variables) + expect(cache.redisCache.get).not.toHaveBeenCalled() + }) + + it("gets / sets the cache by cacheKey", async () => { + const cache = getCache() + cache.enableServerSideCache = true + cache.relayCache.get = jest.fn() + const queryId = "ArtistQuery" + const variables = { slug: "picasso" } + const response = { foo: "bar" } + const options = { cacheConfig: { force: false } } + + const expireSpy = jest.fn(() => Promise.resolve()) + cache.redisCache = { + get: () => Promise.resolve(JSON.stringify(response)), + set: () => Promise.resolve(), + expire: expireSpy, + } as any + + cache.set(queryId, variables, [response], options) + const res = await cache.get(queryId, variables) + expect(res).toEqual(response) + expect(expireSpy).toHaveBeenCalledWith( + cache.getCacheKey(queryId, variables), + cache.cacheConfig.ttl + ) + }) + }) + }) + + describe("cache.clear", () => { + it("clears all caches", () => { + const cache = getCache() + cache.enableServerSideCache = true + const relaySpy = jest.fn() + cache.relayCache.clear = relaySpy + const redisSpy = jest.fn() + cache.redisCache.flushall = redisSpy + cache.clear() + expect(relaySpy).toHaveBeenCalled() + expect(redisSpy).toHaveBeenCalled() + }) + }) +})