fix(env): Always rely on isDevelopment & remove isProduction helper (#…

…998) Closes #997 This flips any usage of `isProduction` to `isDevelopment` which means we "default" to production mode if an environment is unset. This is safer that defaulting to development mode. I've removed the `isProduction` helper completely since we should always be using `isDevelopment`.
arcjet · Jun 18, 2024 · 43423c6 · 43423c6
1 parent b7e5715
commit 43423c6
Show file tree

Hide file tree

Showing 7 changed files with 33 additions and 76 deletions.
diff --git a/arcjet-bun/index.ts b/arcjet-bun/index.ts
@@ -13,13 +13,7 @@ import findIP from "@arcjet/ip";
 import ArcjetHeaders from "@arcjet/headers";
 import type { Server } from "bun";
 import { env } from "bun";
-import {
-  baseUrl,
-  isDevelopment,
-  isProduction,
-  logLevel,
-  platform,
-} from "@arcjet/env";
+import { baseUrl, isDevelopment, logLevel, platform } from "@arcjet/env";
 import { Logger } from "@arcjet/logger";
 import { createClient } from "@arcjet/protocol/client.js";
 
@@ -75,7 +69,7 @@ export function createRemoteClient(options?: RemoteClientOptions) {
 
   // The timeout for the Arcjet API in milliseconds. This is set to a low value
   // in production so calls fail open.
-  const timeout = options?.timeout ?? (isProduction(env) ? 500 : 1000);
+  const timeout = options?.timeout ?? (isDevelopment(env) ? 1000 : 500);
 
   // Transport is the HTTP client that the client uses to make requests.
   const transport = createConnectTransport({

diff --git a/arcjet-next/index.ts b/arcjet-next/index.ts
@@ -18,13 +18,7 @@ import arcjet, {
 } from "arcjet";
 import findIP from "@arcjet/ip";
 import ArcjetHeaders from "@arcjet/headers";
-import {
-  baseUrl,
-  isDevelopment,
-  isProduction,
-  logLevel,
-  platform,
-} from "@arcjet/env";
+import { baseUrl, isDevelopment, logLevel, platform } from "@arcjet/env";
 import { Logger } from "@arcjet/logger";
 import { createClient } from "@arcjet/protocol/client.js";
 
@@ -80,7 +74,7 @@ export function createRemoteClient(options?: RemoteClientOptions) {
 
   // The timeout for the Arcjet API in milliseconds. This is set to a low value
   // in production so calls fail open.
-  const timeout = options?.timeout ?? (isProduction(process.env) ? 500 : 1000);
+  const timeout = options?.timeout ?? (isDevelopment(process.env) ? 1000 : 500);
 
   // Transport is the HTTP client that the client uses to make requests.
   // The Connect Node client doesn't work on edge runtimes: https://github.com/bufbuild/connect-es/pull/589

diff --git a/arcjet-node/index.ts b/arcjet-node/index.ts
@@ -10,13 +10,7 @@ import core, {
 } from "arcjet";
 import findIP from "@arcjet/ip";
 import ArcjetHeaders from "@arcjet/headers";
-import {
-  baseUrl,
-  isDevelopment,
-  isProduction,
-  logLevel,
-  platform,
-} from "@arcjet/env";
+import { baseUrl, isDevelopment, logLevel, platform } from "@arcjet/env";
 import { Logger } from "@arcjet/logger";
 import { createClient } from "@arcjet/protocol/client.js";
 
@@ -72,7 +66,7 @@ export function createRemoteClient(options?: RemoteClientOptions) {
 
   // The timeout for the Arcjet API in milliseconds. This is set to a low value
   // in production so calls fail open.
-  const timeout = options?.timeout ?? (isProduction(process.env) ? 500 : 1000);
+  const timeout = options?.timeout ?? (isDevelopment(process.env) ? 1000 : 500);
 
   // Transport is the HTTP client that the client uses to make requests.
   const transport = createConnectTransport({

diff --git a/arcjet-sveltekit/index.ts b/arcjet-sveltekit/index.ts
@@ -12,13 +12,7 @@ import core, {
 import findIP from "@arcjet/ip";
 import ArcjetHeaders from "@arcjet/headers";
 import { runtime } from "@arcjet/runtime";
-import {
-  baseUrl,
-  isDevelopment,
-  isProduction,
-  logLevel,
-  platform,
-} from "@arcjet/env";
+import { baseUrl, isDevelopment, logLevel, platform } from "@arcjet/env";
 import { Logger } from "@arcjet/logger";
 import { createClient } from "@arcjet/protocol/client.js";
 
@@ -102,7 +96,7 @@ export function createRemoteClient(options?: RemoteClientOptions) {
 
   // The timeout for the Arcjet API in milliseconds. This is set to a low value
   // in production so calls fail open.
-  const timeout = options?.timeout ?? (isProduction(process.env) ? 500 : 1000);
+  const timeout = options?.timeout ?? (isDevelopment(process.env) ? 1000 : 500);
 
   // Transport is the HTTP client that the client uses to make requests.
   const transport = defaultTransport(url);

diff --git a/env/README.md b/env/README.md
@@ -45,11 +45,6 @@ import * as env from "@arcjet/env";
 env.platform({ FLY_APP_NAME: "foobar" }) === "fly-io";
 env.platform({}) === undefined;
 
-env.isProduction({ NODE_ENV: "production" }) === true;
-env.isProduction({ NODE_ENV: "development" }) === false;
-env.isProduction({ ARCJET_ENV: "production" }) === true;
-env.isProduction({ ARCJET_ENV: "development" }) === false;
-
 env.isDevelopment({ NODE_ENV: "production" }) === false;
 env.isDevelopment({ NODE_ENV: "development" }) === true;
 env.isDevelopment({ ARCJET_ENV: "production" }) === false;

diff --git a/env/index.ts b/env/index.ts
@@ -18,10 +18,6 @@ export function isDevelopment(env: Env) {
   return env.NODE_ENV === "development" || env.ARCJET_ENV === "development";
 }
 
-export function isProduction(env: Env) {
-  return env.NODE_ENV === "production" || env.ARCJET_ENV === "production";
-}
-
 export function logLevel(env: Env) {
   const level = env["ARCJET_LOG_LEVEL"];
   switch (level) {
@@ -45,14 +41,9 @@ const baseUrlAllowed = [
 ];
 
 export function baseUrl(env: Env) {
-  // TODO(#90): Remove this production conditional before 1.0.0
-  if (isProduction(env)) {
-    // Use ARCJET_BASE_URL if it is set and belongs to our allowlist; otherwise
-    // use the hardcoded default.
-    if (
-      typeof env["ARCJET_BASE_URL"] === "string" &&
-      baseUrlAllowed.includes(env["ARCJET_BASE_URL"])
-    ) {
+  // TODO(#90): Remove this conditional before 1.0.0
+  if (isDevelopment(env)) {
+    if (env["ARCJET_BASE_URL"]) {
       return env["ARCJET_BASE_URL"];
     }
 
@@ -64,7 +55,12 @@ export function baseUrl(env: Env) {
 
     return "https://decide.arcjet.com";
   } else {
-    if (env["ARCJET_BASE_URL"]) {
+    // Use ARCJET_BASE_URL if it is set and belongs to our allowlist; otherwise
+    // use the hardcoded default.
+    if (
+      typeof env["ARCJET_BASE_URL"] === "string" &&
+      baseUrlAllowed.includes(env["ARCJET_BASE_URL"])
+    ) {
       return env["ARCJET_BASE_URL"];
     }
 

diff --git a/env/test/env.test.ts b/env/test/env.test.ts
@@ -16,14 +16,6 @@ describe("env", () => {
     expect(env.isDevelopment({ ARCJET_ENV: "development" })).toEqual(true);
   });
 
-  test("isProduction", () => {
-    expect(env.isProduction({})).toEqual(false);
-    expect(env.isProduction({ NODE_ENV: "production" })).toEqual(true);
-    expect(env.isProduction({ NODE_ENV: "development" })).toEqual(false);
-    expect(env.isProduction({ ARCJET_ENV: "production" })).toEqual(true);
-    expect(env.isProduction({ ARCJET_ENV: "development" })).toEqual(false);
-  });
-
   test("logLevel", () => {
     expect(env.logLevel({})).toEqual("warn");
     expect(env.logLevel({ ARCJET_LOG_LEVEL: "" })).toEqual("warn");
@@ -36,53 +28,51 @@ describe("env", () => {
 
   test("baseUrl", () => {
     // dev
-    expect(env.baseUrl({})).toEqual("https://decide.arcjet.com");
-    expect(env.baseUrl({ ARCJET_BASE_URL: "anything-in-dev" })).toEqual(
-      "anything-in-dev",
-    );
-    expect(env.baseUrl({ FLY_APP_NAME: "" })).toEqual(
+    expect(env.baseUrl({ NODE_ENV: "development" })).toEqual(
       "https://decide.arcjet.com",
     );
-    expect(env.baseUrl({ FLY_APP_NAME: "foobar" })).toEqual(
-      "https://fly.decide.arcjet.com",
-    );
-    // prod
-    expect(env.baseUrl({ NODE_ENV: "production" })).toEqual(
+    expect(
+      env.baseUrl({
+        NODE_ENV: "development",
+        ARCJET_BASE_URL: "anything-in-dev",
+      }),
+    ).toEqual("anything-in-dev");
+    expect(env.baseUrl({ NODE_ENV: "development", FLY_APP_NAME: "" })).toEqual(
       "https://decide.arcjet.com",
     );
+    expect(
+      env.baseUrl({ NODE_ENV: "development", FLY_APP_NAME: "foobar" }),
+    ).toEqual("https://fly.decide.arcjet.com");
+    // prod
+    expect(env.baseUrl({})).toEqual("https://decide.arcjet.com");
     expect(
       env.baseUrl({
-        NODE_ENV: "production",
         ARCJET_BASE_URL: "https://decide.arcjet.com",
       }),
     ).toEqual("https://decide.arcjet.com");
     expect(
       env.baseUrl({
-        NODE_ENV: "production",
         ARCJET_BASE_URL: "https://decide.arcjettest.com",
       }),
     ).toEqual("https://decide.arcjettest.com");
     expect(
       env.baseUrl({
-        NODE_ENV: "production",
         ARCJET_BASE_URL: "https://fly.decide.arcjet.com",
       }),
     ).toEqual("https://fly.decide.arcjet.com");
     expect(
       env.baseUrl({
-        NODE_ENV: "production",
         ARCJET_BASE_URL: "https://fly.decide.arcjettest.com",
       }),
     ).toEqual("https://fly.decide.arcjettest.com");
     expect(
       env.baseUrl({
-        NODE_ENV: "production",
         ARCJET_BASE_URL: "https://decide.arcjet.orb.local:4082",
       }),
     ).toEqual("https://decide.arcjet.orb.local:4082");
-    expect(
-      env.baseUrl({ NODE_ENV: "production", FLY_APP_NAME: "foobar" }),
-    ).toEqual("https://fly.decide.arcjet.com");
+    expect(env.baseUrl({ FLY_APP_NAME: "foobar" })).toEqual(
+      "https://fly.decide.arcjet.com",
+    );
   });
 
   test("apiKey", () => {