From f04725a62e18bad98ac56eda1802071e980f9502 Mon Sep 17 00:00:00 2001 From: Dan Kadera Date: Thu, 7 Sep 2023 14:39:47 +0200 Subject: [PATCH] add Express.js forking recipe; rework `onFork` hook to allow forking services to run the forked context inside a callback, thus enabling services to use AsyncLocalStorage --- core/cli/package.json | 4 +- core/cli/src/compiler.ts | 44 ++++++++++------ core/cli/src/definitionScanner.ts | 2 +- core/dicc/package.json | 2 +- core/dicc/src/container.ts | 19 ++++--- core/dicc/src/types.ts | 5 +- docs/README.md | 4 ++ docs/recipes/01-express.md | 88 +++++++++++++++++++++++++++++++ docs/sidebar.md | 2 + tsconfig.json | 2 +- 10 files changed, 142 insertions(+), 30 deletions(-) create mode 100644 docs/recipes/01-express.md diff --git a/core/cli/package.json b/core/cli/package.json index 1668e59..7229306 100644 --- a/core/cli/package.json +++ b/core/cli/package.json @@ -9,7 +9,7 @@ "ioc", "inversion of control" ], - "version": "0.0.36", + "version": "0.0.37", "license": "MIT", "author": { "name": "Dan Kadera", @@ -34,7 +34,7 @@ "dependencies": { "@debugr/console": "^3.0.0-rc.10", "@debugr/core": "^3.0.0-rc.12", - "dicc": "^0.0.27", + "dicc": "^0.0.28", "ts-morph": "^18.0", "typescript": "^5.0", "yaml": "^2.3.1", diff --git a/core/cli/src/compiler.ts b/core/cli/src/compiler.ts index 7c6ea5e..1fb1a5f 100644 --- a/core/cli/src/compiler.ts +++ b/core/cli/src/compiler.ts @@ -261,19 +261,21 @@ export class Compiler { info: CallbackInfo | undefined, decorators: DecoratorInfo[], ): void { - const params = this.compileParameters(info?.parameters ?? []); + const params = ['service', ...this.compileParameters(info?.parameters ?? [])]; const decParams = decorators.map(([,, info]) => this.compileParameters(info.parameters)); - const inject = params.length > 0 || decParams.some((p) => p.length > 0); + const inject = params.length > 1 || decParams.some((p) => p.length > 0); const async = info?.async || decorators.some(([,, info]) => info.async); writer.write(`${hook}: `); writer.conditionalWrite(async, 'async '); - writer.write(`(service`); + writer.write(`(`); + writer.conditionalWrite(hook === 'onFork', 'callback, '); + writer.write(`service`); writer.conditionalWrite(inject, ', di'); writer.write(') => '); if (!decorators.length) { - this.compileCall(writer, join('.', source, path, hook), ['service', ...params]); + this.compileCall(writer, join('.', source, path, hook), ['callback', ...params]); writer.write(',\n'); return; } @@ -281,33 +283,43 @@ export class Compiler { writer.write('{\n'); writer.indent(() => { - let service = 'service'; - if (info) { if (hook === 'onFork') { - writer.write('const fork = '); - service = 'fork ?? service'; + const tmp = new CodeBlockWriter(writer.getOptions()); + tmp.write('async (fork) => {\n'); + tmp.indent(() => { + this.compileDecoratorCalls(tmp, decorators, 'fork ?? service', hook, decParams); + tmp.write('return callback(fork);\n'); + }); + tmp.write('}'); + params.unshift(tmp.toString()); } - this.compileCall(writer, join(' ', info?.async && 'await', join('.', source, path, hook)), ['service', ...params]); + this.compileCall(writer, join(' ', hook === 'onFork' ? 'return' : info.async && 'await', join('.', source, path, hook)), params); writer.write(';\n'); } - for (const [i, [source, path, info]] of decorators.entries()) { - writer.conditionalWrite((i > 0 ? decParams[i - 1] : params).length > 0 || decParams[i].length > 0, '\n'); - this.compileCall(writer, join(' ', info.async && 'await', join('.', source, path, hook)), [service, ...decParams[i]]); - writer.write(';\n'); + if (!info || hook !== 'onFork') { + writer.conditionalWrite(params.length > 1 || decParams[0].length > 0, '\n'); + this.compileDecoratorCalls(writer, decorators, 'service', hook, decParams); } - if (hook === 'onFork') { - writer.conditionalWrite(decParams[decParams.length - 1].length > 0, '\n'); - writer.write(`return ${info ? 'fork' : 'undefined'};\n`); + if (!info && hook === 'onFork') { + writer.write('return callback();\n'); } }); writer.write('},\n'); } + private compileDecoratorCalls(writer: CodeBlockWriter, decorators: DecoratorInfo[], service: string, hook: string, decParams: string[][]): void { + for (const [i, [source, path, info]] of decorators.entries()) { + writer.conditionalWrite(i > 0 && (decParams[i - 1].length > 0 || decParams[i].length > 0), '\n'); + this.compileCall(writer, join(' ', info.async && 'await', join('.', source, path, hook)), [service, ...decParams[i]]); + writer.write(';\n'); + } + } + private compileCall(writer: CodeBlockWriter, expression: string, params: string[]): void { writer.write(expression); writer.write('('); diff --git a/core/cli/src/definitionScanner.ts b/core/cli/src/definitionScanner.ts index 458b4eb..54d5670 100644 --- a/core/cli/src/definitionScanner.ts +++ b/core/cli/src/definitionScanner.ts @@ -275,7 +275,7 @@ export class DefinitionScanner { private resolveServiceHook(definition: ObjectLiteralExpression, hook: string): CallbackInfo | undefined { const hookProp = definition.getProperty(hook); - const info = this.resolveCallbackInfo(hookProp, 1); + const info = this.resolveCallbackInfo(hookProp, hook === 'onFork' ? 2 : 1); if (!info && hookProp) { throw new Error(`Invalid '${hook}' hook, must be a method declaration or property assignment`); diff --git a/core/dicc/package.json b/core/dicc/package.json index bec311d..13a6f91 100644 --- a/core/dicc/package.json +++ b/core/dicc/package.json @@ -9,7 +9,7 @@ "ioc", "inversion of control" ], - "version": "0.0.27", + "version": "0.0.28", "license": "MIT", "author": { "name": "Dan Kadera", diff --git a/core/dicc/src/container.ts b/core/dicc/src/container.ts index bcb7327..5635a2e 100644 --- a/core/dicc/src/container.ts +++ b/core/dicc/src/container.ts @@ -25,7 +25,7 @@ export class Container = {}> { private readonly aliases: Map = new Map(); private readonly globalServices: Store = new Store(); private readonly localServices: AsyncLocalStorage = new AsyncLocalStorage(); - private readonly forkHooks: Map> = new Map(); + private readonly forkHooks: [string, CompiledServiceForkHook][] = []; private readonly creating: Set = new Set(); constructor(definitions: CompiledServiceDefinitionMap) { @@ -101,14 +101,19 @@ export class Container = {}> { async fork(cb: () => Promise): Promise { const parent = this.currentStore; const store = new Store(parent); + const chain = this.forkHooks.reduceRight((next, [id, hook]) => { + return async () => { + const callback = async (fork?: any) => { + fork && store.set(id, fork); + return next(); + }; - for (const [id, hook] of this.forkHooks) { - const fork = await hook(await this.get(id), this); - fork && store.set(id, fork); - } + return hook(callback, await this.get(id), this); + }; + }, (async () => this.localServices.run(store, cb)) as () => Promise); try { - return await this.localServices.run(store, cb); + return await chain(); } finally { for (const [id, service] of store) { const definition = this.definitions.get(id); @@ -137,7 +142,7 @@ export class Container = {}> { for (const [id, definition] of Object.entries(definitions)) { this.definitions.set(id, definition) this.aliases.set(id, [id]); - definition.onFork && this.forkHooks.set(id, definition.onFork); + definition.onFork && this.forkHooks.push([id, definition.onFork]); for (const alias of definition.aliases ?? []) { this.aliases.has(alias) || this.aliases.set(alias, []); diff --git a/core/dicc/src/types.ts b/core/dicc/src/types.ts index 49e4364..aed65cd 100644 --- a/core/dicc/src/types.ts +++ b/core/dicc/src/types.ts @@ -28,7 +28,8 @@ export type IterateResult, K extends keyof export type ServiceScope = 'global' | 'local' | 'private'; export type ServiceHook = (service: T, ...args: any[]) => Promise | void; -export type ServiceForkHook = (service: T, ...args: any[]) => Promise | T | undefined; +export type ServiceForkHook = (callback: ServiceForkCallback, service: T, ...args: any[]) => Promise | unknown; +export type ServiceForkCallback = (fork?: T | undefined) => Promise | R; export type ServiceDefinitionOptions = { factory: Constructor | Factory | T | undefined> | undefined; @@ -69,7 +70,7 @@ export type CompiledAsyncServiceHook = { }; export type CompiledServiceForkHook = {}> = { - (service: T, container: Container): Promise | T | undefined; + (callback: ServiceForkCallback, service: T, container: Container): Promise | unknown; }; export type CompiledServiceDefinitionOptions = {}> = { diff --git a/docs/README.md b/docs/README.md index 6bafc2d..70ddb53 100644 --- a/docs/README.md +++ b/docs/README.md @@ -33,6 +33,9 @@ npm i --save dicc - [Config and compilation][4] - how to configure the compiler, how to compile a container and how to use the container at runtime +## Integration recipes + + - [Express][5] ## Developer documentation @@ -44,3 +47,4 @@ to DICC or to extend it with custom functionality. Coming soon! [2]: user/02-intro-to-dicc.md [3]: user/03-services-and-dependencies.md [4]: user/04-config-and-compilation.md +[5]: recipes/01-express.md diff --git a/docs/recipes/01-express.md b/docs/recipes/01-express.md new file mode 100644 index 0000000..24a002d --- /dev/null +++ b/docs/recipes/01-express.md @@ -0,0 +1,88 @@ +# Using `container.fork()` with Express + +The sadly ubiquitous Express HTTP server doesn't mesh well with async code. +This means some care must be taken when integrating DICC into an Express-based +application, otherwise container forking and local services won't work as +expected. + +## TL;DR - how do I use DICC with Express? + +Just use the following code snippet as your very first middleware: + +```typescript +app.use((req, res, next) => { + container.fork(async () => { + next(); + + if (!res.closed) { + await new Promise((resolve) => { + res.on('close', resolve); + }); + } + }); +}); +``` + +## Why? + +The problem is that the return value of Express middlewares is completely +ignored. This means that any middleware which is `async` will not be properly +awaited, and so calling `next()` in a middleware will return as soon as all +subsequent middlewares run their _synchronous_ code - but tracking the end of +any _asynchronous_ execution spawned from middlewares is impossible from the +point of a preceding middleware. The problem is illustrated by the following +snippet: + +```typescript +import express from 'express'; + +const app = express(); + +app.use(async (req, res, next) => { + console.log('mw 1 start'); + await next(); + console.log('mw 1 end'); +}); + +app.use(async (req, res, next) => { + console.log('mw 2 start'); + await next(); + console.log('mw 2 end'); +}); + +app.use(async (req, res, next) => { + console.log('mw 3 start'); + await new Promise((r) => setTimeout(r, 250)); + res.end('hello world'); + console.log('mw 3 end'); +}); + +app.listen(8000); +``` + +The script will output the following sequence when a request is handled: + +``` +mw 1 start +mw 2 start +mw 3 start +mw 2 end +mw 1 end +mw 3 end +``` + +Notice the first and second middlewares log the end of their execution _before_ +the last middleware finishes executing, even though each middleware awaits the +`next()` call. This means that `mw 1` has no direct way of telling when `mw 3` +(or any other middleware) finished handling the request. If we were to naively +use something like `container.fork(next)` in `mw 1`, the forked context would +only be available during the synchronous part of the subsequent middlewares, +because the `fork()` method awaits the provided callback and cleans up the +forked context when the callback resolves - but as we've seen, `next()` doesn't +return a Promise, so it will resolve immediately when all synchronous code has +been executed. + +The snippet at the beginning of this recipe works by waiting for the response +stream to be closed before returning from the fork callback. Unless the app +crashes catastrophically, this will ensure that the forked DI context will stay +alive for the entire duration of the request handling pipeline. diff --git a/docs/sidebar.md b/docs/sidebar.md index 45bedcc..a637e87 100644 --- a/docs/sidebar.md +++ b/docs/sidebar.md @@ -4,6 +4,8 @@ - [Intro to DICC](user/02-intro-to-dicc.md) - [Services and dependencies](user/03-services-and-dependencies.md) - [Config and compilation](user/04-config-and-compilation.md) +- Recipes + - [Express](recipes/01-express.md) - Developer docs - coming soon - [GitHub](https://github.com/cdn77/dicc) diff --git a/tsconfig.json b/tsconfig.json index f53e898..84b316b 100644 --- a/tsconfig.json +++ b/tsconfig.json @@ -9,6 +9,6 @@ "noImplicitThis": true, "noUnusedLocals": true, "strictNullChecks": true, - "target": "ESNext", + "target": "ESNext" } }