next-shared-cache icon indicating copy to clipboard operation
next-shared-cache copied to clipboard

Random route that leads to 404 is cached for 1 year in redis.

Open Vempejul opened this issue 10 months ago • 1 comments

Brief Description of the Bug I am not sure if this is intended or not since I can't seem to find any information about it. We are impelementing a custom cache handler with Azure Cache for Redis. When implementing this I noticed in Redis Insight that all routes that lead to our 404 page are saved and stored in our redis instance with a cache time of 1 year.

Expected vs. Actual Behavior My expectation was that any random route that leads to the 404 page would only cache the 404 page and not also create a key/value for the requested route.

Screenshots/Logs

Image

const { CacheHandler } = require("@neshca/cache-handler")
const createRedisHandler =
  require("@neshca/cache-handler/redis-strings").default
const createLruHandler = require("@neshca/cache-handler/local-lru").default
const { createClient } = require("redis")
const { PHASE_PRODUCTION_BUILD } = require("next/constants")
const { env } = require("./env.mjs")

/* from https://caching-tools.github.io/next-shared-cache/redis */
CacheHandler.onCreation(async () => {
  let client

  // use redis client during build could cause issue https://github.com/caching-tools/next-shared-cache/issues/284#issuecomment-1919145094
  if (
    PHASE_PRODUCTION_BUILD !== process.env.NEXT_PHASE &&
    env.REDIS.url &&
    env.REDIS.password
  ) {
    try {
      // Create a Redis client.
      client = createClient({
        url: env.REDIS.url,
        password: env.REDIS.password,
        socket: {
          tls: true,
        },
      })

      // Redis won't work without error handling.
      // NB do not throw exceptions in the redis error listener,
      // because it will prevent reconnection after a socket exception.
      client.on("error", (e) => {
        if (typeof process.env.NEXT_PRIVATE_DEBUG_CACHE !== "undefined") {
          console.warn("Redis error", e)
        }
      })
    } catch (error) {
      console.warn("Failed to create Redis client:", error)
    }
  }

  if (client) {
    try {
      console.info("Connecting Redis client...")

      // Wait for the client to connect.
      // Caveat: This will block the server from starting until the client is connected.
      // And there is no timeout. Make your own timeout if needed.
      await client.connect()
      console.info("Redis client connected.")
    } catch (error) {
      console.warn("Failed to connect Redis client:", error)

      console.warn("Disconnecting the Redis client...")
      // Try to disconnect the client to stop it from reconnecting.
      client
        .disconnect()
        .then(() => {
          console.info("Redis client disconnected.")
        })
        .catch(() => {
          console.warn(
            "Failed to quit the Redis client after failing to connect.",
          )
        })
    }
  }

  /** @type {import("@neshca/cache-handler").Handler | null} */
  let redisHandler = null
  if (client?.isReady) {
    // Create the `redis-stack` Handler if the client is available and connected.
    redisHandler = createRedisHandler({
      client,
      keyPrefix: process.env.BUILD_ID,
      timeoutMs: 5000,
    })
  }

  // Create fallback LRU handler that is used if Redis client is not available.
  // The application will still work, but the cache will be in memory only and not shared.
  const LRUHandler = createLruHandler()

  return {
    handlers: [redisHandler, LRUHandler],
  }
})

module.exports = CacheHandler

Environment:

  • OS: Windows
  • Node.js version: 22.0.0
  • @neshca/cache-handler version: 1.9.0
  • next version: 14.2.13

Dependencies and Versions List any relevant dependencies and their versions.

Attempted Solutions or Workarounds Describe any attempted solutions or workarounds and their outcomes.

Impact of the Bug Unnecessary usage of storage data and cluttered interface when inspecting the cached resources.

Vempejul avatar Feb 04 '25 10:02 Vempejul

Facing this too. This is causing unnecessary storage usage on my Redis server. I feel like routes that don't exist shouldn't be cached

AkshatKejriwal avatar Mar 24 '25 18:03 AkshatKejriwal