Fb/fixes after trails (#97)

* silent client should disable reconnect strategy for quick connection check

* no need for localhost and default port in default client options

* better handling of default-env

* fix jsdocs order

* update snaps

* easier to read code

* changelog for fixes

Author: rlindner81

CHANGELOG.md

@@ -9,6 +9,14 @@ and this project adheres to [Semantic Versioning](http://semver.org/spec/v2.0.0.
 
 ## v1.2.5 - tbd
 
+### Fixed
+
+- core: if a `./default-env.json` file was present and `NODE_ENV !== "production"`, we had assumed that cf env variables
+  `VCAP_APPLCATION` and `VCAP_SERVICES` are always present in the file. now this is more resilient.
+
+- core: [regression] initialize timing was noticeably slower, because the initial connection check was running with the
+  default reconnect strategy that takes 2000ms. now the reconnect strategy is disabled for the initial connection check.
+
 ## v1.2.4 - 2025-02-19
 
 ### Changed

src/redis-adapter.js

@@ -23,6 +23,11 @@ const INTEGRATION_MODE = Object.freeze({
   LOCAL_REDIS: "LOCAL_REDIS",
   NO_REDIS: "NO_REDIS",
 });
+const CLIENT_NAME = Object.freeze({
+  SILENT: "SILENT",
+  MAIN: "MAIN",
+  SUBSCRIBER: "SUBSCRIBER",
+});
 const CF_REDIS_SERVICE_LABEL = "redis-cache";
 const REDIS_CLIENT_DEFAULT_PING_INTERVAL = 4 * 60000;
 
@@ -88,10 +93,6 @@ const _getRedisOptionsTuple = () => {
   if (!__activeOptionsTuple) {
     const defaultClientOptions = {
       pingInterval: REDIS_CLIENT_DEFAULT_PING_INTERVAL,
-      socket: {
-        host: "localhost",
-        port: 6379,
-      },
     };
 
     const credentials = __customCredentials || cfEnv.cfServiceCredentialsForLabel(CF_REDIS_SERVICE_LABEL);
@@ -100,11 +101,11 @@ const _getRedisOptionsTuple = () => {
     const isCluster = !!credentials.cluster_mode;
     const credentialClientOptions = hasCredentials
       ? {
-          password: credentials.password,
+          ...(Object.prototype.hasOwnProperty.call(credentials, "password") && { password: credentials.password }),
           socket: {
-            host: credentials.hostname,
-            port: credentials.port,
-            tls: credentials.tls,
+            ...(Object.prototype.hasOwnProperty.call(credentials, "hostname") && { host: credentials.hostname }),
+            ...(Object.prototype.hasOwnProperty.call(credentials, "port") && { port: credentials.port }),
+            ...(Object.prototype.hasOwnProperty.call(credentials, "tls") && { tls: credentials.tls }),
           },
         }
       : undefined;
@@ -140,18 +141,28 @@ const _getRedisOptionsTuple = () => {
   return __activeOptionsTuple;
 };
 
+const _noReconnectStrategy = () => new VError({ name: VERROR_CLUSTER_NAME }, "disabled reconnect");
+
 /**
  * Lazily create a new redis client. Client creation transparently handles
  * - custom credentials and client options passed in via {@link setCustomOptions},
  * - Cloud Foundry service with label "redis-cache" (hyperscaler option), and
  * - local redis-server.
  *
+ * @param {string} clientName
+ * @param {CreateOptions} [options]
  * @returns {RedisClient|RedisCluster}
  * @private
  */
-const _createClientBase = (clientName) => {
+const _createClientBase = (clientName, options = {}) => {
+  const { doReconnect = true } = options;
   try {
     const [isCluster, redisClientOptions] = _getRedisOptionsTuple();
+
+    if (!doReconnect) {
+      redisClientOptions.socket.reconnectStrategy = _noReconnectStrategy;
+    }
+
     return isCluster
       ? redis.createCluster({
           rootNodes: [redisClientOptions], // NOTE: assume this ignores everything but socket/url
@@ -167,10 +178,21 @@ const _createClientBase = (clientName) => {
   }
 };
 
-const _createClientAndConnect = async (clientName, { doLogEvents = true } = {}) => {
+/**
+ * @typedef CreateOptions
+ * @type object
+ * @property {boolean}  [doLogEvents] log error events, defaults to true
+ * @property {boolean}  [doReconnect] setting to false disables reconnect strategy, defaults to true
+ */
+/**
+ * @param {string} clientName
+ * @param {CreateOptions} [options]
+ */
+const _createClientAndConnect = async (clientName, options = {}) => {
+  const { doLogEvents = true } = options;
   let client = null;
   try {
-    client = _createClientBase(clientName);
+    client = _createClientBase(clientName, options);
   } catch (err) {
     throw new VError({ name: VERROR_CLUSTER_NAME, cause: err, info: { clientName } }, "error during create client");
   }
@@ -210,7 +232,10 @@ const _canGetClient = async () => {
   if (__canGetClientPromise === null) {
     __canGetClientPromise = (async () => {
       try {
-        const silentClient = await _createClientAndConnect("silent", { doLogEvents: false });
+        const silentClient = await _createClientAndConnect(CLIENT_NAME.SILENT, {
+          doLogEvents: false,
+          doReconnect: false,
+        });
         await _closeClientBase(silentClient);
         return true;
       } catch {} // eslint-disable-line no-empty
@@ -251,7 +276,7 @@ const getIntegrationMode = async () => {
  */
 const getMainClient = async () => {
   if (!__mainClientPromise) {
-    __mainClientPromise = _createClientAndConnect("main");
+    __mainClientPromise = _createClientAndConnect(CLIENT_NAME.MAIN);
   }
   return await __mainClientPromise;
 };
@@ -277,7 +302,7 @@ const closeMainClient = async () => {
  */
 const getSubscriberClient = async () => {
   if (!__subscriberClientPromise) {
-    __subscriberClientPromise = _createClientAndConnect("subscriber");
+    __subscriberClientPromise = _createClientAndConnect(CLIENT_NAME.SUBSCRIBER);
   }
   return await __subscriberClientPromise;
 };

src/shared/cf-env.js

@@ -1,5 +1,8 @@
 "use strict";
 
+const pathlib = require("path");
+const fs = require("fs");
+
 const ENV = Object.freeze({
   USER: "USER",
   CF_APP: "VCAP_APPLICATION",
@@ -23,18 +26,26 @@ class CfEnv {
     } catch (err) {} // eslint-disable-line no-empty
   }
 
-  constructor(env = process.env) {
-    if (env.NODE_ENV !== "production" && env.USE_DEFAULT_ENV) {
-      try {
-        const { VCAP_APPLICATION, VCAP_SERVICES } = require(process.cwd() + "/default-env.json");
-        if (VCAP_APPLICATION && !Object.prototype.hasOwnProperty.call(env, "VCAP_APPLICATION")) {
-          env.VCAP_APPLICATION = JSON.stringify(VCAP_APPLICATION);
-        }
-        if (VCAP_SERVICES && !Object.prototype.hasOwnProperty.call(env, "VCAP_SERVICES")) {
-          env.VCAP_SERVICES = JSON.stringify(VCAP_SERVICES);
+  static readDefaultEnv() {
+    try {
+      const defaultEnvRaw = fs.readFileSync(pathlib.resolve(process.cwd(), "default-env.json"), "utf8");
+      return Object.entries(JSON.parse(defaultEnvRaw)).reduce((acc, [key, value]) => {
+        if (value !== null) {
+          acc[key] = JSON.stringify(value);
         }
-      } catch (err) {} // eslint-disable-line no-empty
-    }
+        return acc;
+      }, {});
+    } catch (err) {} // eslint-disable-line no-empty
+  }
+
+  constructor(inputEnv = process.env) {
+    const env =
+      inputEnv.NODE_ENV === "production"
+        ? inputEnv
+        : {
+            ...CfEnv.readDefaultEnv(),
+            ...inputEnv,
+          };
 
     this.isOnCf = env[ENV.USER] === "vcap";
     this.cfApp = CfEnv.parseEnvVar(env, ENV.CF_APP) || {};

test/redis-adapter.test.js

@@ -74,10 +74,7 @@ describe("redis-adapter test", () => {
       [
         {
           "pingInterval": 240000,
-          "socket": {
-            "host": "localhost",
-            "port": 6379,
-          },
+          "socket": {},
         },
       ]
     `);