Skip to content

Commit b101e20

Browse files
authored
Merge pull request #23 from trivenay/main
fix: context was cleared prematurely in InvokeStoreSingle with async functions
2 parents db30e3f + 5aee9a2 commit b101e20

File tree

4 files changed

+58
-31
lines changed

4 files changed

+58
-31
lines changed

.changeset/fix-async-context.md

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
---
2+
"@aws/lambda-invoke-store": patch
3+
---
4+
5+
Fix context cleared prematurely in InvokeStoreSingle with async functions. Removed try-finally block that was clearing context before async operations completed.
Lines changed: 27 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,27 @@
1+
import { describe, it, expect, beforeEach } from "vitest";
2+
import { InvokeStore, InvokeStoreBase } from "./invoke-store";
3+
4+
describe("InvokeStore - Async Context Bug", () => {
5+
let invokeStore: InvokeStoreBase;
6+
7+
beforeEach(async () => {
8+
if (InvokeStore._testing) {
9+
InvokeStore._testing.reset();
10+
}
11+
invokeStore = await InvokeStore.getInstanceAsync();
12+
});
13+
14+
it("should not clear context after await in InvokeStoreSingle", async () => {
15+
const testContext = {
16+
[InvokeStoreBase.PROTECTED_KEYS.REQUEST_ID]: "test-123",
17+
};
18+
19+
await invokeStore.run(testContext, async () => {
20+
expect(invokeStore.getRequestId()).toBe("test-123");
21+
22+
await new Promise((resolve) => setTimeout(resolve, 10));
23+
24+
expect(invokeStore.getRequestId()).toBe("test-123");
25+
});
26+
});
27+
});

src/invoke-store.spec.ts

Lines changed: 25 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -21,15 +21,6 @@ describe.each([
2121
invokeStore = await InvokeStore.getInstanceAsync();
2222

2323
describe("getRequestId and getXRayTraceId", () => {
24-
it("should return placeholder when called outside run context", () => {
25-
// WHEN
26-
const requestId = invokeStore.getRequestId();
27-
const traceId = invokeStore.getXRayTraceId();
28-
29-
// THEN
30-
expect(requestId).toBe("-");
31-
expect(traceId).toBeUndefined();
32-
});
3324

3425
it("should return current invoke IDs when called within run context", async () => {
3526
// WHEN
@@ -100,12 +91,31 @@ describe.each([
10091
});
10192

10293
describe("getContext", () => {
103-
it("should return undefined when outside run context", () => {
104-
// WHEN
105-
const context = invokeStore.getContext();
94+
it("should replace context on subsequent run calls", async () => {
95+
// WHEN - First run
96+
await invokeStore.run(
97+
{
98+
[InvokeStoreBase.PROTECTED_KEYS.REQUEST_ID]: "first-id",
99+
},
100+
() => {
101+
expect(invokeStore.getRequestId()).toBe("first-id");
102+
},
103+
);
106104

107-
// THEN
108-
expect(context).toBeUndefined();
105+
// WHEN - Second run should replace context
106+
await invokeStore.run(
107+
{
108+
[InvokeStoreBase.PROTECTED_KEYS.REQUEST_ID]: "second-id",
109+
},
110+
() => {
111+
// THEN - Should have new context, not old one
112+
expect(invokeStore.getRequestId()).toBe("second-id");
113+
const context = invokeStore.getContext();
114+
expect(context).toEqual({
115+
[InvokeStoreBase.PROTECTED_KEYS.REQUEST_ID]: "second-id",
116+
});
117+
},
118+
);
109119
});
110120

111121
it("should return complete context with Lambda and custom fields", async () => {
@@ -133,14 +143,6 @@ describe.each([
133143
});
134144

135145
describe("hasContext", () => {
136-
it("should return false when outside run context", () => {
137-
// WHEN
138-
const hasContext = invokeStore.hasContext();
139-
140-
// THEN
141-
expect(hasContext).toBe(false);
142-
});
143-
144146
it("should return true when inside run context", async () => {
145147
// WHEN
146148
const result = await invokeStore.run(
@@ -158,7 +160,7 @@ describe.each([
158160
});
159161

160162
describe("error handling", () => {
161-
it("should propagate errors while maintaining isolation", async () => {
163+
it("should propagate errors", async () => {
162164
// GIVEN
163165
const error = new Error("test error");
164166

@@ -174,7 +176,6 @@ describe.each([
174176

175177
// THEN
176178
await expect(promise).rejects.toThrow(error);
177-
expect(invokeStore.getRequestId()).toBe("-");
178179
});
179180

180181
it("should handle errors in concurrent executions independently", async () => {
@@ -205,7 +206,6 @@ describe.each([
205206
// THEN
206207
expect(traces).toContain("success-success-id");
207208
expect(traces).toContain("before-error-error-id");
208-
expect(invokeStore.getRequestId()).toBe("-");
209209
});
210210
});
211211

@@ -242,7 +242,6 @@ describe.each([
242242

243243
// THEN
244244
await expect(promise).rejects.toThrow(error);
245-
expect(invokeStore.getRequestId()).toBe("-");
246245
});
247246
});
248247
});

src/invoke-store.ts

Lines changed: 1 addition & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -92,11 +92,7 @@ class InvokeStoreSingle extends InvokeStoreBase {
9292

9393
run<T>(context: Context, fn: () => T): T {
9494
this.currentContext = context;
95-
try {
96-
return fn();
97-
} finally {
98-
this.currentContext = undefined;
99-
}
95+
return fn();
10096
}
10197
}
10298

0 commit comments

Comments
 (0)