Async local storage example as CLS replacement

Author: marleystipich2

ASYNC_LOCAL_STORAGE_MIGRATION.md

@@ -0,0 +1,176 @@
+# AsyncLocalStorage Migration Plan
+
+This document outlines the minimal changes needed to switch from `cls-hooked` / `express-http-context` to Node.js built-in `AsyncLocalStorage`.
+
+## Summary
+
+- **Files to modify:** 2
+- **Lines changed:** ~20
+- **No changes needed to:** Connectors, controllers, handlers, resolvers, tests
+
+---
+
+## File 1: `src/util/cls.js`
+
+### BEFORE (current):
+```javascript
+'use strict';
+
+const httpContext = require('express-http-context');
+
+const P = require('bluebird');
+const sequelize = require('sequelize');
+const clsBluebird = require('cls-bluebird');
+clsBluebird(httpContext.ns, P);
+sequelize.useCLS(httpContext.ns);
+
+exports.middleware = function (req, res, next) {
+    httpContext.set('req', req);
+    next();
+};
+
+exports.getReq = function () {
+    return httpContext.get('req');
+};
+
+exports.patchMiddleware = function (fn) {
+    return function (req, res, next) {
+        return fn(req, res, httpContext.ns.bind(next));
+    };
+};
+```
+
+### AFTER (AsyncLocalStorage):
+```javascript
+'use strict';
+
+const { AsyncLocalStorage } = require('async_hooks');
+
+const asyncLocalStorage = new AsyncLocalStorage();
+
+// NOTE: Removed sequelize.useCLS() - not needed because we pass transactions explicitly
+// NOTE: Removed cls-bluebird - not needed with AsyncLocalStorage
+
+exports.middleware = function (req, res, next) {
+    asyncLocalStorage.run({ req }, next);
+};
+
+exports.getReq = function () {
+    const store = asyncLocalStorage.getStore();
+    return store ? store.req : undefined;
+};
+
+exports.patchMiddleware = function (fn) {
+    // AsyncLocalStorage automatically propagates context - no manual binding needed
+    return fn;
+};
+```
+
+---
+
+## File 2: `src/routes.js`
+
+### BEFORE (current):
+```javascript
+// Line 8
+const httpContext = require('express-http-context');
+
+// Lines 66-67
+app.use(httpContext.middleware);
+app.use(cls.middleware);
+```
+
+### AFTER:
+```javascript
+// Line 8 - REMOVE THIS LINE
+// const httpContext = require('express-http-context');
+
+// Lines 66-67 - REMOVE httpContext.middleware, keep cls.middleware
+// app.use(httpContext.middleware);  // DELETE THIS LINE
+app.use(cls.middleware);              // KEEP THIS LINE
+```
+
+---
+
+## Packages that can be removed from package.json (optional, can do later):
+
+```json
+{
+  "dependencies": {
+    "cls-bluebird": "...",        // Can remove
+    "express-http-context": "..."  // Can remove
+  }
+}
+```
+
+Note: Keep these packages for now until you verify everything works. Remove in a follow-up PR.
+
+---
+
+## Why this works:
+
+1. **AsyncLocalStorage is built into Node.js** (since v12.17) - no external package needed
+2. **Works with native Promises** - compatible with axios
+3. **`cls.getReq()` still works** - same API, different implementation
+4. **Sequelize doesn't need CLS** - your code already passes `{transaction}` explicitly (see verification below)
+5. **No connector/controller changes needed** - they keep calling `this.getForwardedHeaders()` which calls `cls.getReq()` internally
+
+---
+
+## Verification: Sequelize transactions are passed explicitly
+
+The `sequelize.useCLS()` feature auto-binds transactions to queries within a transaction callback. However, this codebase **does not rely on it** - all transactions are passed explicitly.
+
+### Transaction blocks and their operations:
+
+| Location | Operations | Transaction passed? |
+|----------|-----------|---------------------|
+| `exports.create` (L167) | `db.remediation.create`, `storeNewActions` | ✓ Yes |
+| `exports.patch` (L210) | `db.remediation.findOne`, `storeNewActions`, `.save` | ✓ Yes |
+| `exports.patchIssue` (L263) | `db.issue.findOne`, `.save`, `remediationUpdated` | ✓ Yes |
+| `findAndDestroy` (L313) | `findOne`, `.destroy`, `remediationUpdated` | ✓ Yes |
+| `findAllAndDestroy` (L334) | `findAll`, `.destroy`, `remediationUpdated` | ✓ Yes |
+
+### Helper functions that receive transaction:
+
+- `storeNewActions(remediation, add, transaction)` - all db operations pass `{transaction}`
+- `remediationUpdated(req, transaction)` - passes `{transaction}` to its update
+
+### Operations outside transactions (no transaction needed):
+
+These are single atomic operations that don't require transactional consistency:
+
+- `insertRHCPlaybookRun` - single create
+- `insertDispatcherRuns` - single bulkCreate  
+- `updateDispatcherRuns` - single update
+- `storePlaybookDefinition` - single create
+
+### Files checked:
+
+- `src/remediations/controller.write.js` - all transaction blocks verified
+- `src/remediations/remediations.queries.js` - no transactions (single operations)
+- `src/generator/generator.controller.js` - no transactions (single operations)
+- `src/admin/admin.controller.js` - no transactions
+
+**Conclusion:** Removing `sequelize.useCLS()` is safe - it was never being relied upon
+
+---
+
+## Testing:
+
+After making these changes:
+1. Run the test suite: `npm test`
+2. Verify HTTP context is available in connectors (headers are forwarded correctly)
+3. Verify database transactions still work (they use explicit `{transaction}` already)
+
+---
+
+## Comparison with explicit `req` passing approach:
+
+| Approach | Files Changed | Complexity |
+|----------|---------------|------------|
+| AsyncLocalStorage | ~2 files | Simple |
+| Explicit `req` passing | ~50+ files | Complex but more explicit |
+
+Both approaches work. AsyncLocalStorage is simpler but keeps implicit context.
+Explicit `req` passing is more work but results in cleaner architecture.

src/routes.js

@@ -5,7 +5,6 @@ const express = require('express');
 const log = require('./util/log');
 const pinoHttp = require('pino-http');
 const prettyJson = require('./middleware/prettyJson');
-const httpContext = require('express-http-context');
 const identity = require('./middleware/identity/impl');
 const userIdentity = require('./middleware/identity/userIdentity');
 const identitySwitcher = require('./middleware/identity/switcher');
@@ -63,7 +62,6 @@ module.exports = async function (app) {
     });
     /*eslint-enable no-unused-vars*/
 
-    app.use(httpContext.middleware);
     app.use(cls.middleware);
     app.use(prettyJson);
 

src/util/cls.js

@@ -1,24 +1,18 @@
 'use strict';
 
-const httpContext = require('express-http-context');
+const { AsyncLocalStorage } = require('async_hooks');
 
-const P = require('bluebird');
-const sequelize = require('sequelize');
-const clsBluebird = require('cls-bluebird');
-clsBluebird(httpContext.ns, P);
-sequelize.useCLS(httpContext.ns);
+const asyncLocalStorage = new AsyncLocalStorage();
 
 exports.middleware = function (req, res, next) {
-    httpContext.set('req', req);
-    next();
+    asyncLocalStorage.run({ req }, next);
 };
 
 exports.getReq = function () {
-    return httpContext.get('req');
+    const store = asyncLocalStorage.getStore();
+    return store ? store.req : undefined;
 };
 
 exports.patchMiddleware = function (fn) {
-    return function (req, res, next) {
-        return fn(req, res, httpContext.ns.bind(next));
-    };
+    return fn;
 };

src/util/cls.unit.js

@@ -0,0 +1,78 @@
+'use strict';
+
+const cls = require('./cls');
+
+describe('AsyncLocalStorage context isolation', function () {
+    test('maintains separate context for concurrent requests', async () => {
+        const results = [];
+
+        // Simulate 3 concurrent requests with different identities
+        const request1 = new Promise(resolve => {
+            const mockReq1 = { id: 'request-1', identity: { user: 'alice' } };
+            cls.middleware(mockReq1, {}, async () => {
+                // Simulate async work
+                await new Promise(r => setTimeout(r, 50));
+                results.push({ expected: 'request-1', actual: cls.getReq().id });
+                resolve();
+            });
+        });
+
+        const request2 = new Promise(resolve => {
+            const mockReq2 = { id: 'request-2', identity: { user: 'bob' } };
+            cls.middleware(mockReq2, {}, async () => {
+                // Simulate async work (shorter delay)
+                await new Promise(r => setTimeout(r, 20));
+                results.push({ expected: 'request-2', actual: cls.getReq().id });
+                resolve();
+            });
+        });
+
+        const request3 = new Promise(resolve => {
+            const mockReq3 = { id: 'request-3', identity: { user: 'charlie' } };
+            cls.middleware(mockReq3, {}, async () => {
+                // Simulate async work (longest delay)
+                await new Promise(r => setTimeout(r, 80));
+                results.push({ expected: 'request-3', actual: cls.getReq().id });
+                resolve();
+            });
+        });
+
+        await Promise.all([request1, request2, request3]);
+
+        // Each request should have received its own context, not another's
+        results.forEach(result => {
+            expect(result.actual).toBe(result.expected);
+        });
+    });
+
+    test('context is undefined outside middleware', () => {
+        expect(cls.getReq()).toBeUndefined();
+    });
+
+    test('nested async operations preserve context', async () => {
+        const mockReq = { id: 'nested-test', data: 'test-value' };
+
+        await new Promise(resolve => {
+            cls.middleware(mockReq, {}, async () => {
+                // Level 1
+                expect(cls.getReq().id).toBe('nested-test');
+
+                await Promise.all([
+                    // Level 2 - parallel promises
+                    (async () => {
+                        await new Promise(r => setTimeout(r, 10));
+                        expect(cls.getReq().id).toBe('nested-test');
+                    })(),
+                    (async () => {
+                        await new Promise(r => setTimeout(r, 20));
+                        expect(cls.getReq().id).toBe('nested-test');
+                    })()
+                ]);
+
+                // After parallel work
+                expect(cls.getReq().id).toBe('nested-test');
+                resolve();
+            });
+        });
+    });
+});