Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Experimental getAsyncCtx() #776

Merged
merged 23 commits into from
Dec 12, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
23 commits
Select commit Hold shift + click to select a range
d1b0b54
Use ALS for retrieving run context
jpwilliams Dec 10, 2024
b09cae8
Export `getAsyncCtx` within `"inngest/experimental"`
jpwilliams Dec 10, 2024
fecd8b3
Set Hono/CFW compat mode
jpwilliams Dec 10, 2024
982c1b0
Revert "Set Hono/CFW compat mode"
jpwilliams Dec 10, 2024
493be7c
Make `node:async_hooks` import safer
jpwilliams Dec 10, 2024
8238151
Fix `@inngest/test` automatic spying not accounting for `step.**`
jpwilliams Dec 11, 2024
104fa51
Move `@inngest/test` package out of `dist/` for easier referencing
jpwilliams Dec 11, 2024
1da3262
Use `@inngest/test@workspace:^` for internal testing
jpwilliams Dec 11, 2024
22765b7
Fix bad `inngest` referencing
jpwilliams Dec 11, 2024
c44a9e6
Add ALS tests
jpwilliams Dec 11, 2024
6b7b9a3
Create cyan-sheep-train.md
jpwilliams Dec 11, 2024
bc31746
Create ten-rockets-float.md
jpwilliams Dec 11, 2024
6982f8a
Create silver-dolls-mix.md
jpwilliams Dec 11, 2024
dee32d2
Create dull-students-wink.md
jpwilliams Dec 11, 2024
e8c02dc
Ensure test dependencies are built when setting up CI
jpwilliams Dec 11, 2024
1e57bc9
Fix silly test type
jpwilliams Dec 11, 2024
d265244
Make sure we do a final install to link any pacages
jpwilliams Dec 11, 2024
ec58bbd
Try using only `@local`
jpwilliams Dec 11, 2024
bafe07c
Ugly fix for in-package tests
jpwilliams Dec 11, 2024
82752ca
Ensure test deps are used during `@inngest/middleware-validation` CI
jpwilliams Dec 11, 2024
cf2da79
Revert workspace rearrange
jpwilliams Dec 11, 2024
0d73a20
Merge branch 'main' into experimental/hooks
jpwilliams Dec 11, 2024
0796d70
Merge branch 'main' into experimental/hooks
jpwilliams Dec 11, 2024
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions .changeset/cyan-sheep-train.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
"inngest": patch
---

Use `@inngest/test@workspace:^` internally for testing
5 changes: 5 additions & 0 deletions .changeset/dull-students-wink.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
"@inngest/test": patch
---

Altered exports to now be namespaced by `./dist/`; if you have directly imported files from `@inngest/test`, you may need to change the imports
11 changes: 11 additions & 0 deletions .changeset/silver-dolls-mix.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
---
"inngest": minor
---

Add experimental `getAsyncCtx()`, allowing the retrieval of a run's input (`event`, `step`, `runId`, etc) from the relevant async chain.

```ts
import { getAsyncCtx } from "inngest/experimental";

const ctx = await getAsyncCtx();
```
8 changes: 7 additions & 1 deletion .github/actions/setup-and-build/action.yml
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,13 @@ runs:
if: ${{ inputs.install-dependencies == 'true' }}
run: pnpm install
shell: bash
working-directory: ${{ inputs.working-directory }}/packages/inngest
working-directory: ${{ inputs.working-directory }}

- name: Build test dependencies
if: ${{ inputs.install-dependencies == 'true' }}
run: pnpm run build
shell: bash
working-directory: ${{ inputs.working-directory }}/packages/test
Comment on lines +41 to +45
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

inngest now relies on @inngest/test@workspace:^, so we make sure that's built during CI too.


- name: Build
if: ${{ inputs.build == 'true' }}
Expand Down
2 changes: 1 addition & 1 deletion packages/inngest/jest.config.js
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ module.exports = {
roots: ["<rootDir>/src"],
moduleNameMapper: {
"(\\..+)\\.js": "$1",
inngest: "<rootDir>/src",
"^inngest$": "<rootDir>/src",
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This regex was picking up anything containing inngest and redirecting it to src/ - bit of a problem when importing @inngest/test!

"^@local$": "<rootDir>/src",
"^@local/(.*)": "<rootDir>/src/$1",
"^@local/(.*)\\.js": "<rootDir>/src/$1",
Expand Down
3 changes: 2 additions & 1 deletion packages/inngest/jsr.json
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@
],
"exports": {
".": "./src/index.ts",
"./experimental": "./src/experimental.ts",
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A new entrypoint to mark out experimental APIs.

"./astro": "./src/astro.ts",
"./bun": "./src/bun.ts",
"./cloudflare": "./src/cloudflare.ts",
Expand All @@ -37,4 +38,4 @@
"./nitro": "./src/nitro.ts",
"./types": "./src/types.ts"
}
}
}
6 changes: 6 additions & 0 deletions packages/inngest/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,11 @@
"import": "./index.js",
"types": "./index.d.ts"
},
"./experimental": {
"require": "./experimental.js",
"import": "./experimental.js",
"types": "./experimental.d.ts"
},
"./astro": {
"require": "./astro.js",
"import": "./astro.js",
Expand Down Expand Up @@ -199,6 +204,7 @@
"@actions/core": "^1.10.0",
"@actions/exec": "^1.1.1",
"@inngest/eslint-plugin-internal": "workspace:^",
"@inngest/test": "workspace:^",
"@jest/globals": "^29.5.0",
"@shopify/jest-koa-mocks": "^5.1.1",
"@sveltejs/kit": "^1.27.3",
Expand Down
155 changes: 155 additions & 0 deletions packages/inngest/src/components/execution/als.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,155 @@
import { InngestTestEngine } from "@inngest/test";
import { type AsyncContext } from "@local/components/execution/als";

describe("getAsyncLocalStorage", () => {
const warningSpy = jest.spyOn(console, "warn");

afterEach(() => {
jest.unmock("node:async_hooks");
jest.resetModules();
});

test("should return an `AsyncLocalStorageIsh`", async () => {
const mod = await import("@local/components/execution/als");
const als = await mod.getAsyncLocalStorage();

expect(als).toBeDefined();
expect(als.getStore).toBeDefined();
expect(als.run).toBeDefined();
});

test("should return the same instance of `AsyncLocalStorageIsh`", async () => {
const mod = await import("@local/components/execution/als");

const als1p = mod.getAsyncLocalStorage();
const als2p = mod.getAsyncLocalStorage();

const als1 = await als1p;
const als2 = await als2p;

expect(als1).toBe(als2);
});

test("should return `undefined` if node:async_hooks is not supported", async () => {
jest.mock("node:async_hooks", () => {
throw new Error("import failed");
});

const mod = await import("@local/components/execution/als");
const als = await mod.getAsyncLocalStorage();

expect(warningSpy).toHaveBeenCalledWith(
expect.stringContaining(
"node:async_hooks is not supported in this runtime"
)
);

expect(als).toBeDefined();
expect(als.getStore()).toBeUndefined();
expect(als.run).toBeDefined();
});
});

describe("getAsyncCtx", () => {
const wait = async () => {
await new Promise((resolve) => setTimeout(resolve));
await new Promise((resolve) => process.nextTick(resolve));
};

afterEach(() => {
jest.unmock("node:async_hooks");
jest.resetModules();
});

test("should return `undefined` outside of an Inngest async context", async () => {
const mod = await import("@local/components/execution/als");
const store = await mod.getAsyncCtx();

expect(store).toBeUndefined();
});

test("should return the input context during execution", async () => {
const { Inngest } = await import("@local");
const mod = await import("@local/experimental");

const inngest = new Inngest({ id: "test" });

// eslint-disable-next-line @typescript-eslint/no-explicit-any
let resolve: (value: any) => void | PromiseLike<void>;
const externalP = new Promise<AsyncContext | undefined>((r) => {
resolve = r;
});

let internalRunId: string | undefined;

const fn = inngest.createFunction(
{ id: "test" },
{ event: "" },
({ runId }) => {
internalRunId = runId;

void wait()
.then(() => mod.getAsyncCtx())
.then(resolve);

return "done";
}
);

// eslint-disable-next-line @typescript-eslint/no-unsafe-assignment, @typescript-eslint/no-explicit-any
const t = new InngestTestEngine({ function: fn as any });

const { result } = await t.execute();

expect(result).toBe("done");
expect(internalRunId).toBeTruthy();

const store = await externalP;
expect(store).toBeDefined();
expect(store?.ctx.runId).toBe(internalRunId);
});

test("should return `undefined` if node:async_hooks is not supported", async () => {
jest.mock("node:async_hooks", () => {
throw new Error("import failed");
});

const { Inngest } = await import("@local");
const mod = await import("@local/experimental");

const inngest = new Inngest({ id: "test" });

// eslint-disable-next-line @typescript-eslint/no-explicit-any
let resolve: (value: any) => void | PromiseLike<void>;
const externalP = new Promise<AsyncContext | undefined>((r) => {
resolve = r;
});

let internalRunId: string | undefined;

const fn = inngest.createFunction(
{ id: "test" },
{ event: "" },
({ runId }) => {
internalRunId = runId;

void wait()
.then(() => mod.getAsyncCtx())
.then(resolve);

return "done";
}
);

// eslint-disable-next-line @typescript-eslint/no-unsafe-assignment, @typescript-eslint/no-explicit-any
const t = new InngestTestEngine({ function: fn as any });

const { result } = await t.execute();

expect(result).toBe("done");
expect(internalRunId).toBeTruthy();

const store = await externalP;
expect(store).toBeUndefined();
});
});
52 changes: 52 additions & 0 deletions packages/inngest/src/components/execution/als.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,52 @@
import { type Context } from "../../types.js";

export interface AsyncContext {
ctx: Context.Any;
}
Comment on lines +3 to +5
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If found, this is the value returned when a developer calls getAsyncCtx() in userland code. It will likely expand to contain more information over time.


/**
* A type that represents a partial, runtime-agnostic interface of
* `AsyncLocalStorage`.
*/
type AsyncLocalStorageIsh = {
getStore: () => AsyncContext | undefined;
run: <R>(store: AsyncContext, fn: () => R) => R;
};

/**
* A local-only variable to store the async local storage instance.
*/
let als: Promise<AsyncLocalStorageIsh> | undefined;

/**
* Retrieve the async context for the current execution.
*/
export const getAsyncCtx = async (): Promise<AsyncContext | undefined> => {
return getAsyncLocalStorage().then((als) => als.getStore());
};
Comment on lines +21 to +26
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is the only piece exposed to the public API, used to retrieve the input to the current function run.


/**
* Get a singleton instance of `AsyncLocalStorage` used to store and retrieve
* async context for the current execution.
*/
export const getAsyncLocalStorage = async (): Promise<AsyncLocalStorageIsh> => {
// eslint-disable-next-line @typescript-eslint/no-misused-promises, no-async-promise-executor
als ??= new Promise<AsyncLocalStorageIsh>(async (resolve) => {
try {
const { AsyncLocalStorage } = await import("node:async_hooks");
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Bun, Deno, and Node all expose this as node:async_hooks.

Cloudflare Workers requires a compatibility flag of node_compat or node_compat_v2. A singular node_als should be possible but doesn't currently work with any version of Wrangler.

In the case that these flags have not been specified, this AsyncLocalStorage instance will essentially become a no-op; no store will ever be returned and .run() will just run the function given directly.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice!

On a side note, how many lines of code would it take you to do this in go!?

als ??= new Promise<AsyncLocalStorageIsh>(async (resolve) => ...


resolve(new AsyncLocalStorage<AsyncContext>());
} catch (err) {
console.warn(
"node:async_hooks is not supported in this runtime. Experimental async context is disabled."
);

resolve({
getStore: () => undefined,
run: (_, fn) => fn(),
});
}
});

return als;
};
71 changes: 38 additions & 33 deletions packages/inngest/src/components/execution/v1.ts
Original file line number Diff line number Diff line change
Expand Up @@ -48,6 +48,7 @@ import {
type InngestExecutionOptions,
type MemoizedOp,
} from "./InngestExecution.js";
import { getAsyncLocalStorage } from "./als.js";

export const createV1InngestExecution: InngestExecutionFactory = (options) => {
return new V1InngestExecution(options);
Expand Down Expand Up @@ -459,44 +460,48 @@ class V1InngestExecution extends InngestExecution implements IInngestExecution {
* and middleware hooks where appropriate.
*/
private async startExecution(): Promise<void> {
/**
* Mutate input as neccessary based on middleware.
*/
await this.transformInput();
return getAsyncLocalStorage().then((als) =>
als.run({ ctx: this.fnArg }, async (): Promise<void> => {
Comment on lines 462 to +464
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

When executing, we now wrap the entire operation in our AsyncLocalStorage.run() call. This also ensures that middleware is included in the async chain.

/**
* Mutate input as neccessary based on middleware.
*/
await this.transformInput();

/**
* Start the timer to time out the run if needed.
*/
void this.timeout?.start();
/**
* Start the timer to time out the run if needed.
*/
void this.timeout?.start();

await this.state.hooks?.beforeMemoization?.();
await this.state.hooks?.beforeMemoization?.();

/**
* If we had no state to begin with, immediately end the memoization phase.
*/
if (this.state.allStateUsed()) {
await this.state.hooks?.afterMemoization?.();
await this.state.hooks?.beforeExecution?.();
}
/**
* If we had no state to begin with, immediately end the memoization phase.
*/
if (this.state.allStateUsed()) {
await this.state.hooks?.afterMemoization?.();
await this.state.hooks?.beforeExecution?.();
}

/**
* Trigger the user's function.
*/
runAsPromise(() => this.userFnToRun(this.fnArg))
// eslint-disable-next-line @typescript-eslint/no-misused-promises
.finally(async () => {
await this.state.hooks?.afterMemoization?.();
await this.state.hooks?.beforeExecution?.();
await this.state.hooks?.afterExecution?.();
})
.then((data) => {
// eslint-disable-next-line @typescript-eslint/no-unsafe-assignment
this.state.setCheckpoint({ type: "function-resolved", data });
/**
* Trigger the user's function.
*/
runAsPromise(() => this.userFnToRun(this.fnArg))
// eslint-disable-next-line @typescript-eslint/no-misused-promises
.finally(async () => {
await this.state.hooks?.afterMemoization?.();
await this.state.hooks?.beforeExecution?.();
await this.state.hooks?.afterExecution?.();
})
.then((data) => {
// eslint-disable-next-line @typescript-eslint/no-unsafe-assignment
this.state.setCheckpoint({ type: "function-resolved", data });
})
.catch((error) => {
// eslint-disable-next-line @typescript-eslint/no-unsafe-assignment
this.state.setCheckpoint({ type: "function-rejected", error });
});
})
.catch((error) => {
// eslint-disable-next-line @typescript-eslint/no-unsafe-assignment
this.state.setCheckpoint({ type: "function-rejected", error });
});
);
}

/**
Expand Down
2 changes: 2 additions & 0 deletions packages/inngest/src/experimental.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
export { getAsyncCtx } from "./components/execution/als.js";
export type { AsyncContext } from "./components/execution/als.js";
2 changes: 1 addition & 1 deletion packages/middleware-validation/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,7 @@
},
"devDependencies": {
"@eslint/js": "^9.7.0",
"@inngest/test": "0.1.1-pr-741.0",
"@inngest/test": "^0.1.3",
"@types/eslint__js": "^8.42.3",
"@types/jest": "^29.5.14",
"eslint": "^8.30.0",
Expand Down
Loading
Loading