From d1df39337e853823b76da935853d20a7dde88db9 Mon Sep 17 00:00:00 2001 From: Jack Williams <1736957+jpwilliams@users.noreply.github.com> Date: Wed, 14 Feb 2024 17:08:25 +0000 Subject: [PATCH 01/10] Add `INNGEST_DEV` toggle for forcing Development Mode --- packages/inngest/etc/inngest.api.md | 11 +-- packages/inngest/src/components/Inngest.ts | 18 ++++ .../src/components/InngestCommHandler.ts | 10 ++- packages/inngest/src/helpers/consts.ts | 1 + packages/inngest/src/helpers/env.ts | 82 +++++++++++++++++-- packages/inngest/src/types.ts | 10 +++ 6 files changed, 117 insertions(+), 15 deletions(-) diff --git a/packages/inngest/etc/inngest.api.md b/packages/inngest/etc/inngest.api.md index e3e45d04..affc7b9d 100644 --- a/packages/inngest/etc/inngest.api.md +++ b/packages/inngest/etc/inngest.api.md @@ -19,6 +19,7 @@ export interface ClientOptions { eventKey?: string; fetch?: typeof fetch; id: string; + isDev?: boolean; // Warning: (ae-forgotten-export) The symbol "Logger" needs to be exported by the entry point index.d.ts logger?: Logger; // Warning: (ae-forgotten-export) The symbol "MiddlewareStack" needs to be exported by the entry point index.d.ts @@ -223,7 +224,7 @@ export enum headerKeys { // @public export class Inngest { - constructor({ id, eventKey, baseUrl, fetch, env, logger, middleware, }: TOpts); + constructor({ id, eventKey, baseUrl, fetch, env, logger, middleware, isDev, }: TOpts); // Warning: (ae-forgotten-export) The symbol "ExclusiveKeys" needs to be exported by the entry point index.d.ts // // (undocumented) @@ -574,9 +575,9 @@ export type ZodEventSchemas = Record { */ private readonly middleware: Promise; + /** + * Whether the client is running in a production environment. This can + * sometimes be `undefined` if the client has expressed no preference or + * perhaps environment variables are only available at a later stage in the + * runtime, for example when receiving a request. + * + * An {@link InngestCommHandler} should prioritize this value over all other + * settings, but should still check for the presence of an environment + * variable if it is not set. + */ + private readonly isProd: Promise; + /** * A client used to interact with the Inngest API by sending or reacting to * events. @@ -159,6 +172,7 @@ export class Inngest { env, logger = new DefaultLogger(), middleware, + isDev, }: TOpts) { if (!id) { // TODO PrettyError @@ -197,6 +211,10 @@ export class Inngest { ...builtInMiddleware, ...(middleware || []), ]); + + this.isProd = isProd({ + explicitSetting: typeof isDev === "boolean" ? !isDev : undefined, + }); } /** diff --git a/packages/inngest/src/components/InngestCommHandler.ts b/packages/inngest/src/components/InngestCommHandler.ts index 1d50702d..b22396f7 100644 --- a/packages/inngest/src/components/InngestCommHandler.ts +++ b/packages/inngest/src/components/InngestCommHandler.ts @@ -670,9 +670,11 @@ export class InngestCommHandler< getInngestHeaders: () => Record; reqArgs: unknown[]; }): Promise { - this._isProd = - (await actions.isProduction?.("starting to handle request")) ?? - isProd(this.env); + this._isProd = await isProd({ + env: this.env, + client: this.client, + actions, + }); /** * If we've been explicitly passed an Inngest dev sever URL, assume that @@ -1408,7 +1410,7 @@ export interface ActionResponse< * This enables us to provide accurate errors for each access without having to * wrap every access in a try/catch. */ -type HandlerResponseWithErrors = { +export type HandlerResponseWithErrors = { [K in keyof HandlerResponse]: NonNullable extends ( ...args: infer Args ) => infer R diff --git a/packages/inngest/src/helpers/consts.ts b/packages/inngest/src/helpers/consts.ts index 92e23368..4b5cb8e7 100644 --- a/packages/inngest/src/helpers/consts.ts +++ b/packages/inngest/src/helpers/consts.ts @@ -31,6 +31,7 @@ export enum envKeys { InngestServePath = "INNGEST_SERVE_PATH", InngestLogLevel = "INNGEST_LOG_LEVEL", InngestStreaming = "INNGEST_STREAMING", + InngestDevMode = "INNGEST_DEV", BranchName = "BRANCH_NAME", diff --git a/packages/inngest/src/helpers/env.ts b/packages/inngest/src/helpers/env.ts index a34d6577..2733c6b3 100644 --- a/packages/inngest/src/helpers/env.ts +++ b/packages/inngest/src/helpers/env.ts @@ -4,6 +4,7 @@ // string in order to read variables. import { type Inngest } from "../components/Inngest"; +import { type HandlerResponseWithErrors } from "../components/InngestCommHandler"; import { type SupportedFrameworkName } from "../types"; import { version } from "../version"; import { envKeys, headerKeys, prodEnvKeys } from "./consts"; @@ -105,16 +106,51 @@ export const skipDevServer = ( }); }; +interface IsProdOptions { + /** + * The optional environment variables to use instead of `process.env`. + */ + env?: Record; + client?: Inngest.Any; + actions?: HandlerResponseWithErrors; + + /** + * If specified as a `boolean`, this will be returned as the result of the + * function. Useful for options that may or may not be set by users. + */ + explicitSetting?: boolean; +} + /** * Returns `true` if we believe the current environment is production based on * either passed environment variables or `process.env`. */ -export const isProd = ( - /** - * The optional environment variables to use instead of `process.env`. - */ - env: Record = allProcessEnv() -): boolean => { +export const isProd = async ({ + env = allProcessEnv(), + client, + actions, + explicitSetting, +}: IsProdOptions = {}): Promise => { + if (typeof explicitSetting === "boolean") { + return explicitSetting; + } + + if (typeof client?.["isProd"] === "boolean") { + return client["isProd"]; + } + + const envIsDev = parseAsBoolean(env[envKeys.InngestDevMode]); + if (typeof envIsDev === "boolean") { + return !envIsDev; + } + + const actionsProd = await actions?.isProduction?.( + "assessing if in production" + ); + if (typeof actionsProd === "boolean") { + return actionsProd; + } + return prodChecks.some(([key, checkKey, expected]) => { return checkFns[checkKey](stringifyUnknown(env[key]), expected); }); @@ -388,3 +424,37 @@ export const getResponse = (): typeof Response => { // eslint-disable-next-line @typescript-eslint/no-unsafe-return, @typescript-eslint/no-unsafe-member-access, @typescript-eslint/no-var-requires return require("cross-fetch").Response; }; + +/** + * Given an unknown value, try to parse it as a `boolean`. Useful for parsing + * environment variables that could be a selection of different values such as + * `"true"`, `"y"`, or `"1"`. + * + * If the value could not be confidently parsed as a `boolean` or was seen to be + * `undefined`, this function returns `undefined`. + */ +export const parseAsBoolean = (value: unknown): boolean | undefined => { + if (typeof value === "boolean") { + return value; + } + + if (typeof value === "number") { + return Boolean(value); + } + + if (typeof value === "string") { + const trimmed = value.trim().toLowerCase(); + + if (trimmed === "undefined") { + return undefined; + } + + if (["true", "1", "y", "yes"].includes(trimmed)) { + return true; + } + + return false; + } + + return undefined; +}; diff --git a/packages/inngest/src/types.ts b/packages/inngest/src/types.ts index 25bca108..80ad774f 100644 --- a/packages/inngest/src/types.ts +++ b/packages/inngest/src/types.ts @@ -573,6 +573,16 @@ export interface ClientOptions { */ logger?: Logger; middleware?: MiddlewareStack; + + /** + * Can be used to explicitly set the client to Development Mode, which will + * turn off signature verification and default to using a local URL to access + * a local Dev Server. + * + * This is useful for forcing the client to use a local Dev Server while also + * running in a production-like environment. + */ + isDev?: boolean; } /** From 0225df8ca7b46bf1618074fb4eb85e2cf759db0f Mon Sep 17 00:00:00 2001 From: Jack Williams <1736957+jpwilliams@users.noreply.github.com> Date: Wed, 14 Feb 2024 17:52:07 +0000 Subject: [PATCH 02/10] Refactor "production" checks to "mode" checks --- packages/inngest/etc/inngest.api.md | 3 +- packages/inngest/src/components/Inngest.ts | 68 ++++++++++------ .../src/components/InngestCommHandler.ts | 44 +++++------ packages/inngest/src/helpers/consts.ts | 2 - packages/inngest/src/helpers/env.ts | 78 ++++++------------- 5 files changed, 93 insertions(+), 102 deletions(-) diff --git a/packages/inngest/etc/inngest.api.md b/packages/inngest/etc/inngest.api.md index affc7b9d..e95b640e 100644 --- a/packages/inngest/etc/inngest.api.md +++ b/packages/inngest/etc/inngest.api.md @@ -266,9 +266,10 @@ export class InngestCommHandler Record): Promise<{ status: number; diff --git a/packages/inngest/src/components/Inngest.ts b/packages/inngest/src/components/Inngest.ts index 29a9b5a5..119c90a4 100644 --- a/packages/inngest/src/components/Inngest.ts +++ b/packages/inngest/src/components/Inngest.ts @@ -12,10 +12,10 @@ import { import { devServerAvailable, devServerUrl } from "../helpers/devserver"; import { getFetch, + getMode, inngestHeaders, - isProd, processEnv, - skipDevServer, + type Mode, } from "../helpers/env"; import { fixEventKeyMissingSteps, prettyError } from "../helpers/errors"; import { stringify } from "../helpers/strings"; @@ -142,7 +142,7 @@ export class Inngest { * settings, but should still check for the presence of an environment * variable if it is not set. */ - private readonly isProd: Promise; + private readonly mode: Mode; /** * A client used to interact with the Inngest API by sending or reacting to @@ -181,15 +181,30 @@ export class Inngest { this.id = id; + this.mode = getMode({ + explicitMode: + typeof isDev === "boolean" ? (isDev ? "dev" : "prod") : undefined, + }); + this.apiBaseUrl = baseUrl || processEnv(envKeys.InngestApiBaseUrl) || - processEnv(envKeys.InngestBaseUrl); + processEnv(envKeys.InngestBaseUrl) || + (this.mode.isExplicit + ? this.mode.type === "prod" + ? defaultInngestApiBaseUrl + : defaultDevServerHost + : undefined); this.eventBaseUrl = baseUrl || processEnv(envKeys.InngestEventApiBaseUrl) || - processEnv(envKeys.InngestBaseUrl); + processEnv(envKeys.InngestBaseUrl) || + (this.mode.isExplicit + ? this.mode.type === "prod" + ? defaultInngestEventBaseUrl + : defaultDevServerHost + : undefined); this.setEventKey(eventKey || processEnv(envKeys.InngestEventKey) || ""); @@ -211,10 +226,6 @@ export class Inngest { ...builtInMiddleware, ...(middleware || []), ]); - - this.isProd = isProd({ - explicitSetting: typeof isDev === "boolean" ? !isDev : undefined, - }); } /** @@ -425,21 +436,9 @@ export class Inngest { let url = this.sendEventUrl.href; /** - * INNGEST_BASE_URL is used to set both dev server and prod URLs, so if a - * user has set this it means they have already chosen a URL to hit. + * If we're in prod mode and have no key, fail now. */ - if (!skipDevServer()) { - if (!this.eventBaseUrl) { - const devAvailable = await devServerAvailable( - defaultDevServerHost, - this.fetch - ); - - if (devAvailable) { - url = devServerUrl(defaultDevServerHost, `e/${this.eventKey}`).href; - } - } - } else if (!this.eventKeySet()) { + if (this.mode.type === "prod" && !this.eventKeySet()) { throw new Error( prettyError({ whatHappened: "Failed to send event", @@ -450,6 +449,29 @@ export class Inngest { ); } + /** + * If we've inferred that we're in dev mode, try to hit the dev server + * first to see if it exists. If it does, use it, otherwise fall back to + * whatever server we have configured. + * + * `INNGEST_BASE_URL` is used to set both dev server and prod URLs, so if a + * user has set this it means they have already chosen a URL to hit. + */ + if ( + this.mode.type === "dev" && + !this.mode.isExplicit && + !this.eventBaseUrl + ) { + const devAvailable = await devServerAvailable( + defaultDevServerHost, + this.fetch + ); + + if (devAvailable) { + url = devServerUrl(defaultDevServerHost, `e/${this.eventKey}`).href; + } + } + const response = await this.fetch(url, { method: "POST", body: stringify(payloads), diff --git a/packages/inngest/src/components/InngestCommHandler.ts b/packages/inngest/src/components/InngestCommHandler.ts index b22396f7..44f2223f 100644 --- a/packages/inngest/src/components/InngestCommHandler.ts +++ b/packages/inngest/src/components/InngestCommHandler.ts @@ -16,11 +16,11 @@ import { allProcessEnv, devServerHost, getFetch, + getMode, inngestHeaders, - isProd, platformSupportsStreaming, - skipDevServer, type Env, + type Mode, } from "../helpers/env"; import { rethrowError, serializeError } from "../helpers/errors"; import { @@ -252,7 +252,7 @@ export class InngestCommHandler< * * Should be set every time a request is received. */ - protected _isProd = false; + protected _mode: Mode | undefined; /** * Whether we should attempt to use the dev server. @@ -670,19 +670,20 @@ export class InngestCommHandler< getInngestHeaders: () => Record; reqArgs: unknown[]; }): Promise { - this._isProd = await isProd({ - env: this.env, - client: this.client, - actions, - }); + const assumedMode = getMode({ env: this.env, client: this.client }); - /** - * If we've been explicitly passed an Inngest dev sever URL, assume that - * we shouldn't skip the dev server. - */ - this._skipDevServer = devServerHost(this.env) - ? false - : this._isProd ?? skipDevServer(this.env); + if (assumedMode.isExplicit) { + this._mode = assumedMode; + } else { + const serveIsProd = await actions.isProduction?.( + "starting to handle request" + ); + if (typeof serveIsProd === "boolean") { + this._mode = { type: serveIsProd ? "prod" : "dev", isExplicit: false }; + } else { + this._mode = assumedMode; + } + } this.upsertKeysFromEnv(); @@ -884,8 +885,7 @@ export class InngestCommHandler< status: 405, body: JSON.stringify({ message: "No action found; request was likely not POST, PUT, or GET", - isProd: this._isProd, - skipDevServer: this._skipDevServer, + mode: this._mode, }), headers: {}, version: undefined, @@ -1139,10 +1139,6 @@ export class InngestCommHandler< return { status, message: error, modified }; } - private get isProd() { - return this._isProd; - } - /** * Given an environment, upsert any missing keys. This is useful in * situations where environment variables are passed directly to handlers or @@ -1171,8 +1167,10 @@ export class InngestCommHandler< } protected validateSignature(sig: string | undefined, body: unknown) { - // Never validate signatures in development. - if (!this.isProd) { + // Never validate signatures outside of prod. Make sure we check the mode + // exists here instead of using nullish coalescing to confirm that the check + // has been completed. + if (this._mode && this._mode.type !== "prod") { return; } diff --git a/packages/inngest/src/helpers/consts.ts b/packages/inngest/src/helpers/consts.ts index 4b5cb8e7..d161ab1b 100644 --- a/packages/inngest/src/helpers/consts.ts +++ b/packages/inngest/src/helpers/consts.ts @@ -98,9 +98,7 @@ export enum envKeys { * {@link https://docs.railway.app/develop/variables#railway-provided-variables} */ RailwayEnvironment = "RAILWAY_ENVIRONMENT", -} -export enum prodEnvKeys { VercelEnvKey = "VERCEL_ENV", } diff --git a/packages/inngest/src/helpers/env.ts b/packages/inngest/src/helpers/env.ts index 2733c6b3..9bab3bbf 100644 --- a/packages/inngest/src/helpers/env.ts +++ b/packages/inngest/src/helpers/env.ts @@ -4,10 +4,9 @@ // string in order to read variables. import { type Inngest } from "../components/Inngest"; -import { type HandlerResponseWithErrors } from "../components/InngestCommHandler"; import { type SupportedFrameworkName } from "../types"; import { version } from "../version"; -import { envKeys, headerKeys, prodEnvKeys } from "./consts"; +import { envKeys, headerKeys } from "./consts"; import { stringifyUnknown } from "./strings"; /** @@ -70,90 +69,63 @@ const prodChecks: [ ["NODE_ENV", "starts with", "prod"], ["VERCEL_ENV", "starts with", "prod"], ["DENO_DEPLOYMENT_ID", "is truthy"], -]; - -// platformDeployChecks are a series of predicates that attempt to check whether -// we're deployed outside of localhost testing. This extends prodChecks with -// platform specific checks, ensuring that if you deploy to eg. vercel without -// NODE_ENV=production we still use prod mode. -const platformDeployChecks: [ - key: string, - customCheck: keyof typeof checkFns, - value?: string, -][] = [ - // Extend prod checks, then check if we're deployed to a platform. - [prodEnvKeys.VercelEnvKey, "is truthy but not", "development"], + [envKeys.VercelEnvKey, "is truthy but not", "development"], [envKeys.IsNetlify, "is truthy"], [envKeys.IsRender, "is truthy"], [envKeys.RailwayBranch, "is truthy"], [envKeys.IsCloudflarePages, "is truthy"], ]; -const skipDevServerChecks = prodChecks.concat(platformDeployChecks); - -/** - * Returns `true` if we're running in production or on a platform, based off of - * either passed environment variables or `process.env`. - */ -export const skipDevServer = ( - /** - * The optional environment variables to use instead of `process.env`. - */ - env: Record = allProcessEnv() -): boolean => { - return skipDevServerChecks.some(([key, checkKey, expected]) => { - return checkFns[checkKey](stringifyUnknown(env[key]), expected); - }); -}; - interface IsProdOptions { /** * The optional environment variables to use instead of `process.env`. */ env?: Record; client?: Inngest.Any; - actions?: HandlerResponseWithErrors; /** * If specified as a `boolean`, this will be returned as the result of the * function. Useful for options that may or may not be set by users. */ - explicitSetting?: boolean; + explicitMode?: Mode["type"]; +} + +export interface Mode { + type: "prod" | "dev"; + + /** + * Whether the mode was explicitly set, or inferred from other sources. + */ + isExplicit: boolean; } /** - * Returns `true` if we believe the current environment is production based on - * either passed environment variables or `process.env`. + * Returns the mode of the current environment, based off of either passed + * environment variables or `process.env`, or explicit settings. */ -export const isProd = async ({ +export const getMode = ({ env = allProcessEnv(), client, - actions, - explicitSetting, -}: IsProdOptions = {}): Promise => { - if (typeof explicitSetting === "boolean") { - return explicitSetting; + explicitMode, +}: IsProdOptions = {}): Mode => { + if (explicitMode) { + return { type: explicitMode, isExplicit: true }; } - if (typeof client?.["isProd"] === "boolean") { - return client["isProd"]; + if (client?.["mode"].isExplicit) { + return client["mode"]; } const envIsDev = parseAsBoolean(env[envKeys.InngestDevMode]); if (typeof envIsDev === "boolean") { - return !envIsDev; - } - - const actionsProd = await actions?.isProduction?.( - "assessing if in production" - ); - if (typeof actionsProd === "boolean") { - return actionsProd; + return { type: envIsDev ? "dev" : "prod", isExplicit: true }; } - return prodChecks.some(([key, checkKey, expected]) => { + const isProd = prodChecks.some(([key, checkKey, expected]) => { return checkFns[checkKey](stringifyUnknown(env[key]), expected); }); + + return { type: isProd ? "prod" : "dev", isExplicit: false }; }; /** From c4f5e852c15ecd49b38198d3da69bdf4fd5b1072 Mon Sep 17 00:00:00 2001 From: Jack Williams <1736957+jpwilliams@users.noreply.github.com> Date: Wed, 14 Feb 2024 18:03:02 +0000 Subject: [PATCH 03/10] Fix some tests that relied on overflowing env vars --- packages/inngest/src/test/helpers.ts | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/packages/inngest/src/test/helpers.ts b/packages/inngest/src/test/helpers.ts index a7593943..ecec781c 100644 --- a/packages/inngest/src/test/helpers.ts +++ b/packages/inngest/src/test/helpers.ts @@ -513,7 +513,7 @@ export const testFramework = ( const ret = await run( [ { - client: new Inngest({ id: "Test", env: "FOO" }), + client: new Inngest({ id: "Test", env: "FOO", isDev: false }), functions: [], }, ], @@ -596,6 +596,7 @@ export const testFramework = ( DENO_DEPLOYMENT_ID: "1", NODE_ENV: "production", ENVIRONMENT: "production", + INNGEST_DEV: "0", }; test("should throw an error in prod with no signature", async () => { const ret = await run( @@ -714,9 +715,7 @@ export const testFramework = ( () => "fn" ); const env = { - DENO_DEPLOYMENT_ID: undefined, - NODE_ENV: "development", - ENVIRONMENT: "development", + INNGEST_DEV: "1", }; test("should throw an error with an invalid JSON body", async () => { From ce18a8205d702f4f88afe7b24cc1ebb8f40a96a5 Mon Sep 17 00:00:00 2001 From: Jack Williams <1736957+jpwilliams@users.noreply.github.com> Date: Thu, 15 Feb 2024 11:22:27 +0000 Subject: [PATCH 04/10] Refactor `"prod"` to `"cloud"` --- packages/inngest/src/components/Inngest.ts | 8 ++++---- packages/inngest/src/components/InngestCommHandler.ts | 4 ++-- packages/inngest/src/helpers/env.ts | 6 +++--- 3 files changed, 9 insertions(+), 9 deletions(-) diff --git a/packages/inngest/src/components/Inngest.ts b/packages/inngest/src/components/Inngest.ts index 119c90a4..7c7864e3 100644 --- a/packages/inngest/src/components/Inngest.ts +++ b/packages/inngest/src/components/Inngest.ts @@ -183,7 +183,7 @@ export class Inngest { this.mode = getMode({ explicitMode: - typeof isDev === "boolean" ? (isDev ? "dev" : "prod") : undefined, + typeof isDev === "boolean" ? (isDev ? "dev" : "cloud") : undefined, }); this.apiBaseUrl = @@ -191,7 +191,7 @@ export class Inngest { processEnv(envKeys.InngestApiBaseUrl) || processEnv(envKeys.InngestBaseUrl) || (this.mode.isExplicit - ? this.mode.type === "prod" + ? this.mode.type === "cloud" ? defaultInngestApiBaseUrl : defaultDevServerHost : undefined); @@ -201,7 +201,7 @@ export class Inngest { processEnv(envKeys.InngestEventApiBaseUrl) || processEnv(envKeys.InngestBaseUrl) || (this.mode.isExplicit - ? this.mode.type === "prod" + ? this.mode.type === "cloud" ? defaultInngestEventBaseUrl : defaultDevServerHost : undefined); @@ -438,7 +438,7 @@ export class Inngest { /** * If we're in prod mode and have no key, fail now. */ - if (this.mode.type === "prod" && !this.eventKeySet()) { + if (this.mode.type === "cloud" && !this.eventKeySet()) { throw new Error( prettyError({ whatHappened: "Failed to send event", diff --git a/packages/inngest/src/components/InngestCommHandler.ts b/packages/inngest/src/components/InngestCommHandler.ts index 44f2223f..68ee8768 100644 --- a/packages/inngest/src/components/InngestCommHandler.ts +++ b/packages/inngest/src/components/InngestCommHandler.ts @@ -679,7 +679,7 @@ export class InngestCommHandler< "starting to handle request" ); if (typeof serveIsProd === "boolean") { - this._mode = { type: serveIsProd ? "prod" : "dev", isExplicit: false }; + this._mode = { type: serveIsProd ? "cloud" : "dev", isExplicit: false }; } else { this._mode = assumedMode; } @@ -1170,7 +1170,7 @@ export class InngestCommHandler< // Never validate signatures outside of prod. Make sure we check the mode // exists here instead of using nullish coalescing to confirm that the check // has been completed. - if (this._mode && this._mode.type !== "prod") { + if (this._mode && this._mode.type !== "cloud") { return; } diff --git a/packages/inngest/src/helpers/env.ts b/packages/inngest/src/helpers/env.ts index 9bab3bbf..0904764a 100644 --- a/packages/inngest/src/helpers/env.ts +++ b/packages/inngest/src/helpers/env.ts @@ -91,7 +91,7 @@ interface IsProdOptions { } export interface Mode { - type: "prod" | "dev"; + type: "cloud" | "dev"; /** * Whether the mode was explicitly set, or inferred from other sources. @@ -118,14 +118,14 @@ export const getMode = ({ const envIsDev = parseAsBoolean(env[envKeys.InngestDevMode]); if (typeof envIsDev === "boolean") { - return { type: envIsDev ? "dev" : "prod", isExplicit: true }; + return { type: envIsDev ? "dev" : "cloud", isExplicit: true }; } const isProd = prodChecks.some(([key, checkKey, expected]) => { return checkFns[checkKey](stringifyUnknown(env[key]), expected); }); - return { type: isProd ? "prod" : "dev", isExplicit: false }; + return { type: isProd ? "cloud" : "dev", isExplicit: false }; }; /** From 61c691afdfa91875f19a973f53755c2d4425de34 Mon Sep 17 00:00:00 2001 From: Jack Williams Date: Mon, 19 Feb 2024 13:19:01 +0000 Subject: [PATCH 05/10] Create many-elephants-smoke.md --- .changeset/many-elephants-smoke.md | 5 +++++ 1 file changed, 5 insertions(+) create mode 100644 .changeset/many-elephants-smoke.md diff --git a/.changeset/many-elephants-smoke.md b/.changeset/many-elephants-smoke.md new file mode 100644 index 00000000..092fa39f --- /dev/null +++ b/.changeset/many-elephants-smoke.md @@ -0,0 +1,5 @@ +--- +"inngest": minor +--- + +INN-2754 Add support for `INNGEST_DEV` and the `isDev` option, allowing a devleoper to explicitly set either Cloud or Dev mode From 7e4617e25a0cbed5db99e47c65476ba96c10f1eb Mon Sep 17 00:00:00 2001 From: Jack Williams <1736957+jpwilliams@users.noreply.github.com> Date: Mon, 19 Feb 2024 14:14:14 +0000 Subject: [PATCH 06/10] Remove `parseAsBoolean()` support for `"y"` and `"yes"` --- packages/inngest/src/helpers/env.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/packages/inngest/src/helpers/env.ts b/packages/inngest/src/helpers/env.ts index 0904764a..2a7b0895 100644 --- a/packages/inngest/src/helpers/env.ts +++ b/packages/inngest/src/helpers/env.ts @@ -400,7 +400,7 @@ export const getResponse = (): typeof Response => { /** * Given an unknown value, try to parse it as a `boolean`. Useful for parsing * environment variables that could be a selection of different values such as - * `"true"`, `"y"`, or `"1"`. + * `"true"`, `"1"`. * * If the value could not be confidently parsed as a `boolean` or was seen to be * `undefined`, this function returns `undefined`. @@ -421,7 +421,7 @@ export const parseAsBoolean = (value: unknown): boolean | undefined => { return undefined; } - if (["true", "1", "y", "yes"].includes(trimmed)) { + if (["true", "1"].includes(trimmed)) { return true; } From 2c83fcf20c62e36b59cc192a182d2d27ab602d3a Mon Sep 17 00:00:00 2001 From: Jack Williams <1736957+jpwilliams@users.noreply.github.com> Date: Mon, 19 Feb 2024 18:24:32 +0000 Subject: [PATCH 07/10] Show `mode` when hitting the `GET` introspection request --- packages/inngest/src/components/InngestCommHandler.ts | 1 + packages/inngest/src/types.ts | 7 +++++++ 2 files changed, 8 insertions(+) diff --git a/packages/inngest/src/components/InngestCommHandler.ts b/packages/inngest/src/components/InngestCommHandler.ts index 68ee8768..50e3e49b 100644 --- a/packages/inngest/src/components/InngestCommHandler.ts +++ b/packages/inngest/src/components/InngestCommHandler.ts @@ -834,6 +834,7 @@ export class InngestCommHandler< hasEventKey: this.client["eventKeySet"](), hasSigningKey: Boolean(this.signingKey), functionsFound: registerBody.functions.length, + mode: this._mode, }; return { diff --git a/packages/inngest/src/types.ts b/packages/inngest/src/types.ts index 80ad774f..13c34d4b 100644 --- a/packages/inngest/src/types.ts +++ b/packages/inngest/src/types.ts @@ -14,6 +14,7 @@ import { } from "./components/InngestMiddleware"; import { type createStepTools } from "./components/InngestStepTools"; import { type internalEvents } from "./helpers/consts"; +import { type Mode } from "./helpers/env"; import { type IsStringLiteral, type ObjectPaths, @@ -1093,6 +1094,12 @@ export interface IntrospectRequest { * The number of Inngest functions found at this handler. */ functionsFound: number; + + /** + * The mode that this handler is running in and whether it has been inferred + * or explicitly set. + */ + mode: Mode; } /** From cba765e96abccdb17da6c3b2ef5ab8fd99afc247 Mon Sep 17 00:00:00 2001 From: Jack Williams <1736957+jpwilliams@users.noreply.github.com> Date: Tue, 20 Feb 2024 12:52:35 +0000 Subject: [PATCH 08/10] Fix always attempting to use the dev server due to an old check --- .../inngest/src/components/InngestCommHandler.ts | 12 ++++-------- 1 file changed, 4 insertions(+), 8 deletions(-) diff --git a/packages/inngest/src/components/InngestCommHandler.ts b/packages/inngest/src/components/InngestCommHandler.ts index 50e3e49b..bca5c4c6 100644 --- a/packages/inngest/src/components/InngestCommHandler.ts +++ b/packages/inngest/src/components/InngestCommHandler.ts @@ -254,13 +254,6 @@ export class InngestCommHandler< */ protected _mode: Mode | undefined; - /** - * Whether we should attempt to use the dev server. - * - * Should be set every time a request is received. - */ - protected _skipDevServer = false; - /** * The localized `fetch` implementation used by this handler. */ @@ -1077,7 +1070,10 @@ export class InngestCommHandler< // is a noop and returns false in production. let registerURL = this.inngestRegisterUrl; - if (!this._skipDevServer) { + const inferredDevMode = + this._mode && !this._mode.isExplicit && this._mode.type === "dev"; + + if (inferredDevMode) { const host = devServerHost(this.env); const hasDevServer = await devServerAvailable(host, this.fetch); if (hasDevServer) { From b48adbf37694467cb12070095660e4c3c61825ce Mon Sep 17 00:00:00 2001 From: Jack Williams <1736957+jpwilliams@users.noreply.github.com> Date: Fri, 23 Feb 2024 12:02:23 +0000 Subject: [PATCH 09/10] Add `INNGEST_DEV=[A_URL]` and address PR notes --- packages/inngest/etc/inngest.api.md | 6 +- packages/inngest/src/components/Inngest.ts | 28 ++--- .../src/components/InngestCommHandler.ts | 15 ++- packages/inngest/src/helpers/env.ts | 115 ++++++++++++++++-- 4 files changed, 123 insertions(+), 41 deletions(-) diff --git a/packages/inngest/etc/inngest.api.md b/packages/inngest/etc/inngest.api.md index 614c8102..b731390f 100644 --- a/packages/inngest/etc/inngest.api.md +++ b/packages/inngest/etc/inngest.api.md @@ -585,9 +585,9 @@ export type ZodEventSchemas = Record { baseUrl || processEnv(envKeys.InngestApiBaseUrl) || processEnv(envKeys.InngestBaseUrl) || - (this.mode.isExplicit - ? this.mode.type === "cloud" - ? defaultInngestApiBaseUrl - : defaultDevServerHost - : undefined); + this.mode.getExplicitUrl(defaultInngestApiBaseUrl); this.eventBaseUrl = baseUrl || processEnv(envKeys.InngestEventApiBaseUrl) || processEnv(envKeys.InngestBaseUrl) || - (this.mode.isExplicit - ? this.mode.type === "cloud" - ? defaultInngestEventBaseUrl - : defaultDevServerHost - : undefined); + this.mode.getExplicitUrl(defaultInngestEventBaseUrl); this.setEventKey(eventKey || processEnv(envKeys.InngestEventKey) || ""); @@ -436,9 +428,9 @@ export class Inngest { let url = this.sendEventUrl.href; /** - * If we're in prod mode and have no key, fail now. + * If in prod mode and key is not present, fail now. */ - if (this.mode.type === "cloud" && !this.eventKeySet()) { + if (this.mode.isCloud && !this.eventKeySet()) { throw new Error( prettyError({ whatHappened: "Failed to send event", @@ -450,18 +442,14 @@ export class Inngest { } /** - * If we've inferred that we're in dev mode, try to hit the dev server - * first to see if it exists. If it does, use it, otherwise fall back to - * whatever server we have configured. + * If dev mode has been inferred, try to hit the dev server first to see if + * it exists. If it does, use it, otherwise fall back to whatever server we + * have configured. * * `INNGEST_BASE_URL` is used to set both dev server and prod URLs, so if a * user has set this it means they have already chosen a URL to hit. */ - if ( - this.mode.type === "dev" && - !this.mode.isExplicit && - !this.eventBaseUrl - ) { + if (this.mode.isDev && this.mode.isInferred && !this.eventBaseUrl) { const devAvailable = await devServerAvailable( defaultDevServerHost, this.fetch diff --git a/packages/inngest/src/components/InngestCommHandler.ts b/packages/inngest/src/components/InngestCommHandler.ts index 695808db..eed8dcfc 100644 --- a/packages/inngest/src/components/InngestCommHandler.ts +++ b/packages/inngest/src/components/InngestCommHandler.ts @@ -13,6 +13,7 @@ import { } from "../helpers/consts"; import { devServerAvailable, devServerUrl } from "../helpers/devserver"; import { + Mode, allProcessEnv, devServerHost, getFetch, @@ -20,7 +21,6 @@ import { inngestHeaders, platformSupportsStreaming, type Env, - type Mode, } from "../helpers/env"; import { rethrowError, serializeError } from "../helpers/errors"; import { @@ -672,7 +672,10 @@ export class InngestCommHandler< "starting to handle request" ); if (typeof serveIsProd === "boolean") { - this._mode = { type: serveIsProd ? "cloud" : "dev", isExplicit: false }; + this._mode = new Mode({ + type: serveIsProd ? "cloud" : "dev", + isExplicit: false, + }); } else { this._mode = assumedMode; } @@ -1090,7 +1093,7 @@ export class InngestCommHandler< let registerURL = new URL(this.inngestRegisterUrl.href); const inferredDevMode = - this._mode && !this._mode.isExplicit && this._mode.type === "dev"; + this._mode && this._mode.isInferred && this._mode.isDev; if (inferredDevMode) { const host = devServerHost(this.env); @@ -1098,6 +1101,8 @@ export class InngestCommHandler< if (hasDevServer) { registerURL = devServerUrl(host, "/fn/register"); } + } else if (this._mode?.explicitDevUrl) { + registerURL = new URL(this._mode.explicitDevUrl); } if (deployId) { @@ -1183,10 +1188,10 @@ export class InngestCommHandler< } protected validateSignature(sig: string | undefined, body: unknown) { - // Never validate signatures outside of prod. Make sure we check the mode + // Never validate signatures outside of prod. Make sure to check the mode // exists here instead of using nullish coalescing to confirm that the check // has been completed. - if (this._mode && this._mode.type !== "cloud") { + if (this._mode && !this._mode.isCloud) { return; } diff --git a/packages/inngest/src/helpers/env.ts b/packages/inngest/src/helpers/env.ts index 2a7b0895..1f748830 100644 --- a/packages/inngest/src/helpers/env.ts +++ b/packages/inngest/src/helpers/env.ts @@ -6,7 +6,7 @@ import { type Inngest } from "../components/Inngest"; import { type SupportedFrameworkName } from "../types"; import { version } from "../version"; -import { envKeys, headerKeys } from "./consts"; +import { defaultDevServerHost, envKeys, headerKeys } from "./consts"; import { stringifyUnknown } from "./strings"; /** @@ -36,13 +36,26 @@ export const devServerHost = (env: Env = allProcessEnv()): EnvValue => { // processed using webpack's DefinePlugin, which is dumb and does a straight // text replacement instead of actually understanding the AST, despite webpack // being fully capable of understanding the AST. - const values = [ - env[envKeys.InngestBaseUrl], - env[`REACT_APP_${envKeys.InngestBaseUrl}`], - env[`NEXT_PUBLIC_${envKeys.InngestBaseUrl}`], - ]; + const prefixes = ["REACT_APP_", "NEXT_PUBLIC_"]; + const keys = [envKeys.InngestBaseUrl, envKeys.InngestDevMode]; - return values.find((a) => !!a); + const values = keys.flatMap((key) => { + return prefixes.map((prefix) => { + return env[prefix + key]; + }); + }); + + return values.find((v) => { + if (!v) { + return; + } + + try { + return Boolean(new URL(v)); + } catch { + // no-op + } + }); }; const checkFns = (< @@ -81,6 +94,12 @@ interface IsProdOptions { * The optional environment variables to use instead of `process.env`. */ env?: Record; + + /** + * The Inngest client that's being used when performing this check. This is + * used to check if the client has an explicit mode set, and if so, to use + * that mode instead of inferring it from the environment. + */ client?: Inngest.Any; /** @@ -90,13 +109,72 @@ interface IsProdOptions { explicitMode?: Mode["type"]; } -export interface Mode { +export interface ModeOptions { type: "cloud" | "dev"; /** * Whether the mode was explicitly set, or inferred from other sources. */ isExplicit: boolean; + + /** + * If the mode was explicitly set as a dev URL, this is the URL that was set. + */ + explicitDevUrl?: URL; +} + +export class Mode { + private readonly type: "cloud" | "dev"; + + /** + * Whether the mode was explicitly set, or inferred from other sources. + */ + public readonly isExplicit: boolean; + + public readonly explicitDevUrl?: URL; + + constructor({ type, isExplicit, explicitDevUrl }: ModeOptions) { + this.type = type; + this.isExplicit = isExplicit || Boolean(explicitDevUrl); + this.explicitDevUrl = explicitDevUrl; + } + + public get isDev(): boolean { + return this.type === "dev"; + } + + public get isCloud(): boolean { + return this.type === "cloud"; + } + + public get isInferred(): boolean { + return !this.isExplicit; + } + + /** + * If we are explicitly in a particular mode, retrieve the URL that we are + * sure we should be using, not considering any environment variables or other + * influences. + */ + public getExplicitUrl(defaultCloudUrl: string): string | undefined { + if (!this.isExplicit) { + return undefined; + } + + if (this.explicitDevUrl) { + return this.explicitDevUrl.href; + } + + if (this.isCloud) { + return defaultCloudUrl; + } + + if (this.isDev) { + return defaultDevServerHost; + } + + return undefined; + } } /** @@ -109,23 +187,34 @@ export const getMode = ({ explicitMode, }: IsProdOptions = {}): Mode => { if (explicitMode) { - return { type: explicitMode, isExplicit: true }; + return new Mode({ type: explicitMode, isExplicit: true }); } if (client?.["mode"].isExplicit) { return client["mode"]; } - const envIsDev = parseAsBoolean(env[envKeys.InngestDevMode]); - if (typeof envIsDev === "boolean") { - return { type: envIsDev ? "dev" : "cloud", isExplicit: true }; + if (envKeys.InngestDevMode in env) { + if (typeof env[envKeys.InngestDevMode] === "string") { + try { + const explicitDevUrl = new URL(env[envKeys.InngestDevMode]); + return new Mode({ type: "dev", isExplicit: true, explicitDevUrl }); + } catch { + // no-op + } + } + + const envIsDev = parseAsBoolean(env[envKeys.InngestDevMode]); + if (typeof envIsDev === "boolean") { + return new Mode({ type: envIsDev ? "dev" : "cloud", isExplicit: true }); + } } const isProd = prodChecks.some(([key, checkKey, expected]) => { return checkFns[checkKey](stringifyUnknown(env[key]), expected); }); - return { type: isProd ? "cloud" : "dev", isExplicit: false }; + return new Mode({ type: isProd ? "cloud" : "dev", isExplicit: false }); }; /** From bc74a5a1ecad7e0d735fd36d7357ce8efdccf257 Mon Sep 17 00:00:00 2001 From: Jack Williams <1736957+jpwilliams@users.noreply.github.com> Date: Fri, 23 Feb 2024 12:28:28 +0000 Subject: [PATCH 10/10] Add `INNGEST_DEV` and `isDev` unit tests --- .../inngest/src/components/Inngest.test.ts | 105 ++++++++++++++++++ 1 file changed, 105 insertions(+) diff --git a/packages/inngest/src/components/Inngest.test.ts b/packages/inngest/src/components/Inngest.test.ts index a6aa4c75..71d5f54d 100644 --- a/packages/inngest/src/components/Inngest.test.ts +++ b/packages/inngest/src/components/Inngest.test.ts @@ -23,6 +23,111 @@ const testEvent: EventPayload = { const testEventKey = "foo-bar-baz-test"; +describe("new Inngest()", () => { + describe("mode", () => { + const createTestClient = ({ + env, + opts, + }: { + env?: Record; + opts?: Omit[0], "id">; + } = {}): Inngest.Any => { + let ogKeys: Record = {}; + + if (env) { + ogKeys = Object.keys(env).reduce>( + (acc, key) => { + acc[key] = process.env[key]; + process.env[key] = env[key]; + return acc; + }, + {} + ); + } + + const inngest = new Inngest({ id: "test", ...opts }); + + if (env) { + Object.keys(ogKeys).forEach((key) => { + process.env[key] = ogKeys[key]; + }); + } + + return inngest; + }; + + test("should default to inferred dev mode", () => { + const inngest = createTestClient(); + expect(inngest["mode"].isDev).toBe(true); + expect(inngest["mode"].isExplicit).toBe(false); + }); + + test("`isDev: true` sets explicit dev mode", () => { + const inngest = createTestClient({ opts: { isDev: true } }); + expect(inngest["mode"].isDev).toBe(true); + expect(inngest["mode"].isExplicit).toBe(true); + }); + + test("`isDev: false` sets explict cloud mode", () => { + const inngest = createTestClient({ opts: { isDev: false } }); + expect(inngest["mode"].isCloud).toBe(true); + expect(inngest["mode"].isExplicit).toBe(true); + }); + + test("`INNGEST_DEV=1 sets explicit dev mode", () => { + const inngest = createTestClient({ + env: { [envKeys.InngestDevMode]: "1" }, + }); + expect(inngest["mode"].isDev).toBe(true); + expect(inngest["mode"].isExplicit).toBe(true); + }); + + test("`INNGEST_DEV=true` sets explicit dev mode", () => { + const inngest = createTestClient({ + env: { [envKeys.InngestDevMode]: "true" }, + }); + expect(inngest["mode"].isDev).toBe(true); + expect(inngest["mode"].isExplicit).toBe(true); + }); + + test("`INNGEST_DEV=false` sets explicit cloud mode", () => { + const inngest = createTestClient({ + env: { [envKeys.InngestDevMode]: "false" }, + }); + expect(inngest["mode"].isCloud).toBe(true); + expect(inngest["mode"].isExplicit).toBe(true); + }); + + test("`INNGEST_DEV=0 sets explicit cloud mode", () => { + const inngest = createTestClient({ + env: { [envKeys.InngestDevMode]: "0" }, + }); + expect(inngest["mode"].isCloud).toBe(true); + expect(inngest["mode"].isExplicit).toBe(true); + }); + + test("`isDev` overwrites `INNGEST_DEV`", () => { + const inngest = createTestClient({ + env: { [envKeys.InngestDevMode]: "1" }, + opts: { isDev: false }, + }); + expect(inngest["mode"].isDev).toBe(false); + expect(inngest["mode"].isExplicit).toBe(true); + }); + + test("`INNGEST_DEV=URL sets explicit dev mode", () => { + const inngest = createTestClient({ + env: { [envKeys.InngestDevMode]: "http://localhost:3000" }, + }); + expect(inngest["mode"].isDev).toBe(true); + expect(inngest["mode"].isExplicit).toBe(true); + expect(inngest["mode"].explicitDevUrl?.href).toBe( + "http://localhost:3000/" + ); + }); + }); +}); + describe("send", () => { describe("runtime", () => { const originalProcessEnv = process.env;