From d0a94e746f90f4a7f3e2643c599cba280275afcc Mon Sep 17 00:00:00 2001 From: Matteo Collina Date: Tue, 21 May 2024 18:18:34 +0200 Subject: [PATCH] Upgrade to latest asyncforge (#11) * Remove the use of enterWith Signed-off-by: Matteo Collina * fixup Signed-off-by: Matteo Collina * fixup Signed-off-by: Matteo Collina * fixup Signed-off-by: Matteo Collina * added store.enterWith() Signed-off-by: Matteo Collina * fixup Signed-off-by: Matteo Collina * updated asyncforge Signed-off-by: Matteo Collina * fixup Signed-off-by: Matteo Collina --------- Signed-off-by: Matteo Collina --- README.md | 66 ++++----------------- example/{app.js => app.mjs} | 16 +++--- example/{do-work.js => do-work.mjs} | 6 +- index.d.ts | 3 +- index.js | 54 +++++++++-------- index.test-d.ts | 2 +- package.json | 2 +- test/async-hooks.test.js | 17 +----- test/basic.test.js | 89 +++++++++++++++++++++-------- test/before.test.js | 41 +++++++++++++ test/issue-7.test.mjs | 54 ----------------- 11 files changed, 162 insertions(+), 188 deletions(-) rename example/{app.js => app.mjs} (61%) rename example/{do-work.js => do-work.mjs} (51%) create mode 100644 test/before.test.js delete mode 100644 test/issue-7.test.mjs diff --git a/README.md b/README.md index 23e7b48..705020d 100644 --- a/README.md +++ b/README.md @@ -12,18 +12,19 @@ npm i fastify fastify-asyncforge ## Usage ```js -// App.js +// app.mjs import fastify from 'fastify' -import { start } from 'fastify-asyncforge' import doWork from './do-work.mjs' +import asyncforge, { logger } from 'fastify-asyncforge' const app = fastify({ logger: true }) +await app.register(asyncforge) -// It's fundamental that `start` is called before any other plugins are registered, otherwise the helpers -// would not work as expected -await start(app) +app.runInAsyncScope(() => { + logger().info('hello') +}) app.decorate('foo', 'bar') app.decorateRequest('a') @@ -54,49 +55,9 @@ export default function doWork () { } ``` -### enterWith - -If you need to alter the current asynchronous context, you can use the `enterWith` helper. - -```js -import { before, describe, it } from "node:test"; -import Fastify from "fastify"; -import asyncforge, { app, start } from "fastify-asyncforge"; -import assert from "node:assert/strict"; - -let fastify; - -async function build(config) { - const server = await Fastify(); - - server.register(asyncforge) - server.decorate("config", config); - console.log("config from memo", app().config); - - return server; -} - -describe("support exiting from a context", () => { - before(async () => { - fastify = await build({ foo: "bar" }); - }); - - it("throws", () => { - assert.throws(app); - }); - - it("does not throw using enterWith", () => { - fastify.enterWith(); - assert.equal(app(), fastify); - }); -}); -``` - -## Usage together with other async_hooks tools (e.g. DataDog, OpenTelemetry, etc) +## `.enterWith()` -If you are using `fastify-asyncforge` together with another `async_hooks` based tool, -you __must__ call `start` without adding an intermediate async function. -Alternatively, you'd need to use `.enterWith()` +In case `.runInAsyncScope()` is incovenient, you can use `.enterWith()` ```js import fastify from 'fastify' @@ -104,19 +65,14 @@ import asyncforge from 'fastify-asyncforge' const fastify = Fastify() -async function wrap () { - // This is necessary to make it throw - await 1 - await asyncforge.start(fastify) -} - -await wrap() - // Calling .enterWith() is necessary or `asyncforge.app()` will throw fastify.enterWith() asyncforge.app() ``` +### Notice + +Note that you cannot wrap `.enterWith()` inside an async function, as it will not work. If you are interested in knowing more, read https://github.com/nodejs/node/issues/53037. ## License diff --git a/example/app.js b/example/app.mjs similarity index 61% rename from example/app.js rename to example/app.mjs index 90232ef..5478401 100644 --- a/example/app.js +++ b/example/app.mjs @@ -1,17 +1,15 @@ -'use strict' - -const fastify = require('fastify') -const doWork = require('./do-work') -const { logger, start } = require('../index') +import fastify from 'fastify' +import doWork from './do-work.mjs' +import asyncforge, { logger } from '../index.js' const app = fastify({ logger: true }) -app.register(require('../index')) - -start(app) +await app.register(asyncforge) -logger().info('hello') +app.runInAsyncScope(() => { + logger().info('hello') +}) app.decorate('foo', 'bar') app.decorateRequest('a') diff --git a/example/do-work.js b/example/do-work.mjs similarity index 51% rename from example/do-work.js rename to example/do-work.mjs index fed2c1b..b57fc13 100644 --- a/example/do-work.js +++ b/example/do-work.mjs @@ -1,8 +1,6 @@ -'use strict' +import { logger, app, request, reply } from '../index.js' -const { logger, app, request, reply } = require('../') - -module.exports = function doWork () { +export default function doWork () { const log = logger() log.info({ foo: app().foo, diff --git a/index.d.ts b/index.d.ts index c425e8e..e9b90c9 100644 --- a/index.d.ts +++ b/index.d.ts @@ -8,7 +8,8 @@ import { declare module "fastify" { interface FastifyInstance { - enterWith: () => void; + runInAsyncScope (fn: () => T) : T; + enterWith() : void } } diff --git a/index.js b/index.js index 28dd03b..7414333 100644 --- a/index.js +++ b/index.js @@ -1,7 +1,7 @@ 'use strict' const fp = require('fastify-plugin') -const { memo, setAll } = require('asyncforge') +const { memo, create } = require('asyncforge') const app = memo('fastify.app') const request = memo('fastify.request') @@ -9,39 +9,43 @@ const reply = memo('fastify.reply') const logger = memo('fastify.logger') const fastifyAsyncForge = fp(function (fastify, opts, next) { + fastify.decorate('runInAsyncScope', function (fn) { + const store = create() + return store.run(() => { + app.set(this) + logger.set(this.log) + return fn() + }) + }) + fastify.decorate('enterWith', function () { - try { - app() - } catch { - setAll({ - [app.key]: this, - [logger.key]: this.log - }) - } + const store = create() + store.enterWith() + app.set(this) + logger.set(this.log) }) - fastify.addHook('onRequest', async function (req, res) { - setAll({ - [app.key]: this, - [request.key]: req, - [reply.key]: res, - [logger.key]: req.log + fastify.addHook('onRequest', function (req, res, next) { + const store = create() + + // We use callbacks because we cannot propagate through async/await + store.run(() => { + app.set(this) + request.set(req) + reply.set(res) + logger.set(req.log) + next() }) }) - next() -}) -function start (fastify) { - setAll({ - [app.key]: fastify, - [logger.key]: fastify.log + create(() => { + app.set(fastify) + logger.set(fastify.log) + next() }) - - return fastify.register(fastifyAsyncForge) -} +}) module.exports = fastifyAsyncForge -module.exports.start = start module.exports.app = app module.exports.request = request module.exports.reply = reply diff --git a/index.test-d.ts b/index.test-d.ts index 0608401..3fc3806 100644 --- a/index.test-d.ts +++ b/index.test-d.ts @@ -14,11 +14,11 @@ const fastifyInstance = fastify(); expectAssignable(fastifyInstance.register(fastifyasyncforge)); expectAssignable(fastifyasyncforge); expectType(fastifyInstance.enterWith()); +expectType(fastifyInstance.runInAsyncScope(() => 42)); // app expectAssignable(fastifyInstance); expectAssignable(app()); -expectType(app().enterWith()); expectAssignable(app>()); expectError(app()); expectError({}); diff --git a/package.json b/package.json index 074b84a..ef2001d 100644 --- a/package.json +++ b/package.json @@ -31,7 +31,7 @@ "tsd": "^0.31.0" }, "dependencies": { - "asyncforge": "^0.2.0", + "asyncforge": "^0.5.0", "fastify-plugin": "^4.5.1" }, "types": "index.d.ts" diff --git a/test/async-hooks.test.js b/test/async-hooks.test.js index 7084571..072ed07 100644 --- a/test/async-hooks.test.js +++ b/test/async-hooks.test.js @@ -16,13 +16,7 @@ test('async_hooks lose context', async (t) => { const fastify = Fastify() t.after(() => fastify.close()) - async function wrap () { - // This is necessary to make it throw - await 1 - await fastifyAsyncForge.start(fastify) - } - - await wrap() + await fastify.register(fastifyAsyncForge) // This is expected to throw due to https://github.com/nodejs/node/issues/53037 p.throws(logger) @@ -51,13 +45,8 @@ test('enterWith fixes it', async (t) => { const fastify = Fastify() t.after(() => fastify.close()) - async function wrap () { - await 1 - await fastifyAsyncForge.start(fastify) - } - - await wrap() - await fastify.enterWith() + await fastify.register(fastifyAsyncForge) + fastify.enterWith() p.strictEqual(logger(), fastify.log) p.strictEqual(app(), fastify) diff --git a/test/basic.test.js b/test/basic.test.js index ea77df1..e3b3352 100644 --- a/test/basic.test.js +++ b/test/basic.test.js @@ -7,14 +7,16 @@ const fastifyAsyncForge = require('../') const { app, request, reply, logger } = fastifyAsyncForge -test('basic helpers with start', async (t) => { +test('basic helpers', async (t) => { const p = tspl(t, { plan: 7 }) const fastify = Fastify() - await fastifyAsyncForge.start(fastify) + await fastify.register(fastifyAsyncForge) - p.strictEqual(logger(), fastify.log) - p.strictEqual(app(), fastify) + fastify.runInAsyncScope(() => { + p.strictEqual(logger(), fastify.log) + p.strictEqual(app(), fastify) + }) fastify.get('/', async function (_request, _reply) { p.strictEqual(app(), this) @@ -34,14 +36,16 @@ test('basic helpers with start', async (t) => { await p.completed }) -test('basic helpers without start', async (t) => { +test('support in following plugin', async (t) => { const p = tspl(t, { plan: 7 }) const fastify = Fastify() - await fastify.register(fastifyAsyncForge) + fastify.register(fastifyAsyncForge) - p.throws(logger) - p.throws(app) + fastify.register(async function (fastify, _opts) { + p.strictEqual(app(), Object.getPrototypeOf(fastify)) + p.strictEqual(request(), undefined) + }) fastify.get('/', async function (_request, _reply) { p.strictEqual(app(), this) @@ -61,16 +65,20 @@ test('basic helpers without start', async (t) => { await p.completed }) -test('fastify.enterWith', async (t) => { - const p = tspl(t, { plan: 7 }) +test('support in following plugin with await ', async (t) => { + const p = tspl(t, { plan: 9 }) const fastify = Fastify() await fastify.register(fastifyAsyncForge) - fastify.enterWith() - - p.strictEqual(logger(), fastify.log) - p.strictEqual(app(), fastify) + fastify.register(async function (fastify, _opts) { + p.throws(app) + p.throws(request) + fastify.runInAsyncScope(() => { + p.strictEqual(app(), fastify) + p.strictEqual(request(), undefined) + }) + }) fastify.get('/', async function (_request, _reply) { p.strictEqual(app(), this) @@ -94,10 +102,12 @@ test('with additional onRequest hook', async (t) => { const p = tspl(t, { plan: 15 }) const fastify = Fastify() - await fastifyAsyncForge.start(fastify) + await fastify.register(fastifyAsyncForge) - p.strictEqual(logger(), fastify.log) - p.strictEqual(app(), fastify) + fastify.runInAsyncScope(() => { + p.strictEqual(logger(), fastify.log) + p.strictEqual(app(), fastify) + }) fastify.addHook('onRequest', async function b (_request, _reply) { p.strictEqual(app(), this) @@ -136,16 +146,18 @@ test('onRequest hook added before registration fails', async (t) => { const fastify = Fastify() fastify.addHook('onRequest', async function b (_request, _reply) { - p.strictEqual(app(), this) - p.strictEqual(logger(), fastify.log) - p.strictEqual(request(), undefined) - p.strictEqual(reply(), undefined) + p.throws(app) + p.throws(logger) + p.throws(request) + p.throws(reply) }) - await fastifyAsyncForge.start(fastify) + await fastify.register(fastifyAsyncForge) - p.strictEqual(logger(), fastify.log) - p.strictEqual(app(), fastify) + fastify.runInAsyncScope(() => { + p.strictEqual(logger(), fastify.log) + p.strictEqual(app(), fastify) + }) fastify.get('/', { onRequest: async function a (_request, _reply) { @@ -171,3 +183,32 @@ test('onRequest hook added before registration fails', async (t) => { await p.completed }) + +test('basic helpers with start', async (t) => { + const p = tspl(t, { plan: 7 }) + const fastify = Fastify() + + await fastify.register(fastifyAsyncForge) + + fastify.runInAsyncScope(() => { + p.strictEqual(logger(), fastify.log) + p.strictEqual(app(), fastify) + }) + + fastify.get('/', async function (_request, _reply) { + p.strictEqual(app(), this) + p.strictEqual(request(), _request) + p.strictEqual(reply(), _reply) + p.strictEqual(logger(), _request.log) + return { hello: 'world' } + }) + + const res = await fastify.inject({ + method: 'GET', + url: '/' + }) + + p.strictEqual(res.statusCode, 200) + + await p.completed +}) diff --git a/test/before.test.js b/test/before.test.js new file mode 100644 index 0000000..b232a06 --- /dev/null +++ b/test/before.test.js @@ -0,0 +1,41 @@ +'use strict' + +const { describe, it, before } = require('node:test') +const assert = require('node:assert') +const Fastify = require('fastify') +const asyncforge = require('../') + +describe('support exiting from a context', () => { + let fastify + before(async () => { + fastify = Fastify() + await fastify.register(asyncforge) + }) + + it('throws', () => { + assert.throws(asyncforge.app) + }) + + it('does not throw using enterWith', () => { + fastify.enterWith() + assert.equal(asyncforge.app(), fastify) + }) +}) + +describe('enterWith in before', () => { + let fastify + before(async () => { + fastify = Fastify() + await fastify.register(asyncforge) + fastify.enterWith() + }) + + it('throws', () => { + assert.throws(asyncforge.app) + }) + + it('does not throw using enterWith again', () => { + fastify.enterWith() + assert.equal(asyncforge.app(), fastify) + }) +}) diff --git a/test/issue-7.test.mjs b/test/issue-7.test.mjs deleted file mode 100644 index cc9806e..0000000 --- a/test/issue-7.test.mjs +++ /dev/null @@ -1,54 +0,0 @@ -import { before, describe, it } from 'node:test' -import Fastify from 'fastify' -import asyncforge, { app, start } from '../index.js' -import assert from 'node:assert/strict' - -let fastify - -async function build (config) { - const server = await Fastify() - - server.decorate('config', config) - await start(server) - - return server -} - -async function buildWithRegister (config) { - const server = await Fastify() - - server.register(asyncforge) - server.decorate('config', config) - - return server -} - -describe('support exiting from a context / start', () => { - before(async () => { - fastify = await build({ foo: 'bar' }) - }) - - it('throws', () => { - assert.throws(app) - }) - - it('does not throw using enterWith', () => { - fastify.enterWith() - assert.equal(app(), fastify) - }) -}) - -describe('support exiting from a context / register', () => { - before(async () => { - fastify = await buildWithRegister({ foo: 'bar' }) - }) - - it('throws', () => { - assert.throws(app) - }) - - it('does not throw using enterWith', () => { - fastify.enterWith() - assert.equal(app(), fastify) - }) -})