Skip to content

Commit efa419a

Browse files
authored
feat(eslint-plugin): add no-closure-in-durable-operations rule (#252)
- Add new rule to detect and prevent closure variable mutations in durable operations (step, runInChildContext, waitForCondition, waitForCallback) - Rule allows reading closure variables but prevents assignments and updates (=, +=, ++, --) - Recursively tracks variable declarations across nested scopes - Reorganize rules into subdirectories with rule implementation and tests co-located - Add unit tests covering mutations, reads, and scope handling - Update README with new rule documentation and examples **Limitations**: 1. Object/Array Mutations: The rule only catches direct assignments and update expressions but misses property mutations: ``` let obj = { count: 0 }; await context.step(async () => { obj.count++; // ❌ Not detected but equally problematic obj.items.push(1); // ❌ Not detected }); ``` 2. Destructuring Assignments: ``` let a = 1, b = 2; await context.step(async () => { [a, b] = [b, a]; // ❌ Not detected }); ``` 3. Performance Consideration: The walkNode function recursively traverses the entire callback AST. For large callbacks, this could be slow. I'll investigate ESLint's built-in visitor pattern instead of manual traversal later. 5. Duplicate Error in Nested Cases: Nested operations report the error twice (once for each level). This might be confusing to users - I'll enhance it later to report at the innermost level.
1 parent 14dca12 commit efa419a

File tree

11 files changed

+1893
-15
lines changed

11 files changed

+1893
-15
lines changed

packages/aws-durable-execution-sdk-js-eslint-plugin/README.md

Lines changed: 77 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -16,7 +16,8 @@ Add the plugin to your ESLint configuration:
1616
{
1717
"plugins": ["@aws/durable-execution-sdk-js-eslint-plugin"],
1818
"rules": {
19-
"@aws/durable-execution-sdk-js-eslint-plugin/no-nested-durable-operations": "error"
19+
"@aws/durable-execution-sdk-js-eslint-plugin/no-nested-durable-operations": "error",
20+
"@aws/durable-execution-sdk-js-eslint-plugin/no-closure-in-durable-operations": "error"
2021
}
2122
}
2223
```
@@ -65,22 +66,93 @@ const result = await context.runInChildContext("block1", async (childCtx) => {
6566
});
6667
```
6768

69+
### `no-closure-in-durable-operations`
70+
71+
Prevents modifying closure variables inside durable operations. Reading closure variables is allowed.
72+
73+
#### ❌ Incorrect
74+
75+
```javascript
76+
const handler = withDurableExecution(
77+
async (event: any, context: DurableContext) => {
78+
let a = 2;
79+
const result = await context.runInChildContext(
80+
async (childContext: DurableContext) => {
81+
const stepResult = await childContext.step(async () => {
82+
// Error: Modifying 'a' from outer scope causes replay inconsistency
83+
a = a + 1;
84+
return "child step completed";
85+
});
86+
return stepResult;
87+
},
88+
);
89+
return result;
90+
},
91+
);
92+
```
93+
94+
#### ✅ Correct
95+
96+
```javascript
97+
const handler = withDurableExecution(
98+
async (event: any, context: DurableContext) => {
99+
let a = 2;
100+
const result = await context.runInChildContext(
101+
async (childContext: DurableContext) => {
102+
const stepResult = await childContext.step(async () => {
103+
// Reading 'a' is OK
104+
const value = a + 1;
105+
return value;
106+
});
107+
return stepResult;
108+
},
109+
);
110+
return result;
111+
},
112+
);
113+
114+
// Or use local variables
115+
const handler = withDurableExecution(
116+
async (event: any, context: DurableContext) => {
117+
const result = await context.runInChildContext(
118+
async (childContext: DurableContext) => {
119+
const stepResult = await childContext.step(async () => {
120+
let a = 2;
121+
a = a + 1;
122+
return "child step completed";
123+
});
124+
return stepResult;
125+
},
126+
);
127+
return result;
128+
},
129+
);
130+
```
131+
68132
## Supported Durable Operations
69133

70134
The plugin detects these durable operations:
71135

72136
- `step`
73-
- `wait`
74-
- `waitForCallback`
137+
- `runInChildContext`
75138
- `waitForCondition`
139+
- `waitForCallback`
140+
- `wait`
76141
- `parallel`
77142
- `map`
78-
- `runInChildContext`
79143

80-
## Why This Rule?
144+
## Why These Rules?
145+
146+
### No Nested Durable Operations
81147

82148
Nesting durable operations with the same context object can cause runtime errors and unexpected behavior in AWS Lambda Durable Functions. This rule helps catch these issues at development time.
83149

150+
### No Closure in Durable Operations
151+
152+
During replay, durable functions skip already-executed steps. If a closure variable is modified inside a step, the modification won't occur during replay, causing different outcomes between initial execution and replay. This leads to non-deterministic behavior and potential bugs.
153+
154+
Reading closure variables is safe because it doesn't change state. Only mutations (assignments, increments, decrements) are problematic.
155+
84156
## License
85157

86158
Apache-2.0

packages/aws-durable-execution-sdk-js-eslint-plugin/jest.config.js

Lines changed: 2 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -2,10 +2,6 @@ module.exports = {
22
preset: "ts-jest",
33
testEnvironment: "node",
44
roots: ["<rootDir>/src"],
5-
testMatch: ["**/__tests__/**/*.test.ts"],
6-
collectCoverageFrom: [
7-
"src/**/*.ts",
8-
"!src/**/*.test.ts",
9-
"!src/**/__tests__/**",
10-
],
5+
testMatch: ["**/*.test.ts"],
6+
collectCoverageFrom: ["src/**/*.ts", "!src/**/*.test.ts"],
117
};

packages/aws-durable-execution-sdk-js-eslint-plugin/src/index.ts

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,17 +1,20 @@
1-
import { noNestedDurableOperations } from "./rules/no-nested-durable-operations";
2-
import { noNonDeterministicOutsideStep } from "./rules/no-non-deterministic-outside-step";
1+
import { noNestedDurableOperations } from "./rules/no-nested-durable-operations/no-nested-durable-operations";
2+
import { noNonDeterministicOutsideStep } from "./rules/no-non-deterministic-outside-step/no-non-deterministic-outside-step";
3+
import { noClosureInDurableOperations } from "./rules/no-closure-in-durable-operations/no-closure-in-durable-operations";
34

45
export = {
56
rules: {
67
"no-nested-durable-operations": noNestedDurableOperations,
78
"no-non-deterministic-outside-step": noNonDeterministicOutsideStep,
9+
"no-closure-in-durable-operations": noClosureInDurableOperations,
810
},
911
configs: {
1012
recommended: {
1113
plugins: ["@aws/durable-functions"],
1214
rules: {
1315
"@aws/durable-functions/no-nested-durable-operations": "error",
1416
"@aws/durable-functions/no-non-deterministic-outside-step": "error",
17+
"@aws/durable-functions/no-closure-in-durable-operations": "error",
1518
},
1619
},
1720
},

0 commit comments

Comments
 (0)