From d278202dd476ca0a89e7576c7c1996c465368e0e Mon Sep 17 00:00:00 2001 From: Hans Ott Date: Mon, 21 Oct 2024 17:13:55 +0200 Subject: [PATCH 1/4] Log warning only once --- library/agent/context/user.test.ts | 109 ++++++++++++++++------------- library/agent/context/user.ts | 8 ++- 2 files changed, 69 insertions(+), 48 deletions(-) diff --git a/library/agent/context/user.test.ts b/library/agent/context/user.test.ts index 14eb59d89..2c447c61a 100644 --- a/library/agent/context/user.test.ts +++ b/library/agent/context/user.test.ts @@ -1,13 +1,17 @@ import * as t from "tap"; -import { type Context, getContext, runWithContext } from "../Context"; +import { wrap } from "../../helpers/wrap"; +import { + type Context, + getContext, + runWithContext, + updateContext, +} from "../Context"; import { LoggerForTesting } from "../logger/LoggerForTesting"; import { setUser } from "./user"; import { createTestAgent } from "../../helpers/createTestAgent"; -createTestAgent(); - -t.test("it does not set user if empty id", async (t) => { - const context: Context = { +function createContext(): Context { + return { remoteAddress: "::1", method: "POST", url: "http://localhost:4000", @@ -21,6 +25,14 @@ t.test("it does not set user if empty id", async (t) => { source: "express", route: "/posts/:id", }; +} + +t.beforeEach(() => { + createTestAgent(); +}); + +t.test("it does not set user if empty id", async (t) => { + const context = createContext(); runWithContext(context, () => { setUser({ id: "" }); @@ -33,20 +45,7 @@ t.test("it does not set user if not inside context", async () => { }); t.test("it sets user", async (t) => { - const context: Context = { - remoteAddress: "::1", - method: "POST", - url: "http://localhost:4000", - query: {}, - headers: {}, - body: { - myTitle: `-- should be blocked`, - }, - cookies: {}, - routeParams: {}, - source: "express", - route: "/posts/:id", - }; + const context = createContext(); runWithContext(context, () => { setUser({ id: "id" }); @@ -57,20 +56,7 @@ t.test("it sets user", async (t) => { }); t.test("it sets user with number as ID", async (t) => { - const context: Context = { - remoteAddress: "::1", - method: "POST", - url: "http://localhost:4000", - query: {}, - headers: {}, - body: { - myTitle: `-- should be blocked`, - }, - cookies: {}, - routeParams: {}, - source: "express", - route: "/posts/:id", - }; + const context = createContext(); runWithContext(context, () => { setUser({ id: 1 }); @@ -81,20 +67,7 @@ t.test("it sets user with number as ID", async (t) => { }); t.test("it sets user with name", async (t) => { - const context: Context = { - remoteAddress: "::1", - method: "POST", - url: "http://localhost:4000", - query: {}, - headers: {}, - body: { - myTitle: `-- should be blocked`, - }, - cookies: {}, - routeParams: {}, - source: "express", - route: "/posts/:id", - }; + const context = createContext(); runWithContext(context, () => { setUser({ id: "id", name: "name" }); @@ -139,3 +112,45 @@ t.test("it logs when setUser has invalid input", async () => { ]); logger.clear(); }); + +t.test( + "it does not log warning when setUser is called before middleware", + async () => { + const logs: string[] = []; + wrap(console, "warn", function warn() { + return function warn(message: string) { + logs.push(message); + }; + }); + + const context = createContext(); + runWithContext(context, () => { + setUser({ id: "id" }); + }); + + t.same(logs, []); + } +); + +t.test( + "it logs warning when setUser is called after middleware (once)", + async () => { + const logs: string[] = []; + wrap(console, "warn", function warn() { + return function warn(message: string) { + logs.push(message); + }; + }); + + const context = createContext(); + runWithContext(context, () => { + updateContext(getContext()!, "executedMiddleware", true); + setUser({ id: "id" }); + setUser({ id: "id" }); + }); + + t.same(logs, [ + "setUser(...) must be called before the Zen middleware is executed.", + ]); + } +); diff --git a/library/agent/context/user.ts b/library/agent/context/user.ts index cf01b4479..ef13a21da 100644 --- a/library/agent/context/user.ts +++ b/library/agent/context/user.ts @@ -3,6 +3,8 @@ import { getInstance } from "../AgentSingleton"; import type { User } from "../Context"; import { ContextStorage } from "./ContextStorage"; +let loggedWarningSetUserCalledAfterMiddleware = false; + export function setUser(user: unknown) { const agent = getInstance(); @@ -48,11 +50,15 @@ export function setUser(user: unknown) { return; } - if (context.executedMiddleware) { + if ( + !loggedWarningSetUserCalledAfterMiddleware && + context.executedMiddleware + ) { // eslint-disable-next-line no-console console.warn( `setUser(...) must be called before the Zen middleware is executed.` ); + loggedWarningSetUserCalledAfterMiddleware = true; } context.user = validatedUser; From 96b0cae1743f32b868632e61381edb21bef27441 Mon Sep 17 00:00:00 2001 From: Hans Ott Date: Mon, 21 Oct 2024 17:18:01 +0200 Subject: [PATCH 2/4] Extract function --- library/agent/context/user.ts | 25 ++++++++++++++++--------- 1 file changed, 16 insertions(+), 9 deletions(-) diff --git a/library/agent/context/user.ts b/library/agent/context/user.ts index ef13a21da..fa539bcc7 100644 --- a/library/agent/context/user.ts +++ b/library/agent/context/user.ts @@ -50,15 +50,9 @@ export function setUser(user: unknown) { return; } - if ( - !loggedWarningSetUserCalledAfterMiddleware && - context.executedMiddleware - ) { - // eslint-disable-next-line no-console - console.warn( - `setUser(...) must be called before the Zen middleware is executed.` - ); - loggedWarningSetUserCalledAfterMiddleware = true; + if (context.executedMiddleware) { + logWarningSetUserCalledAfterMiddleware(); + return; } context.user = validatedUser; @@ -71,3 +65,16 @@ export function setUser(user: unknown) { lastIpAddress: ipAddress, }); } + +function logWarningSetUserCalledAfterMiddleware() { + if (loggedWarningSetUserCalledAfterMiddleware) { + return; + } + + // eslint-disable-next-line no-console + console.warn( + `setUser(...) must be called before the Zen middleware is executed.` + ); + + loggedWarningSetUserCalledAfterMiddleware = true; +} From 8c16592bcf8c549a4e493ac6ca4730509a9d35eb Mon Sep 17 00:00:00 2001 From: Hans Ott Date: Mon, 21 Oct 2024 17:19:29 +0200 Subject: [PATCH 3/4] Move variable closer --- library/agent/context/user.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/library/agent/context/user.ts b/library/agent/context/user.ts index fa539bcc7..58f65b92d 100644 --- a/library/agent/context/user.ts +++ b/library/agent/context/user.ts @@ -3,8 +3,6 @@ import { getInstance } from "../AgentSingleton"; import type { User } from "../Context"; import { ContextStorage } from "./ContextStorage"; -let loggedWarningSetUserCalledAfterMiddleware = false; - export function setUser(user: unknown) { const agent = getInstance(); @@ -66,6 +64,8 @@ export function setUser(user: unknown) { }); } +let loggedWarningSetUserCalledAfterMiddleware = false; + function logWarningSetUserCalledAfterMiddleware() { if (loggedWarningSetUserCalledAfterMiddleware) { return; From 44209793fc5cf95176c0ea82f138a579cd23a7cc Mon Sep 17 00:00:00 2001 From: Hans Ott Date: Mon, 21 Oct 2024 17:32:53 +0200 Subject: [PATCH 4/4] Remove return --- library/agent/context/user.ts | 1 - 1 file changed, 1 deletion(-) diff --git a/library/agent/context/user.ts b/library/agent/context/user.ts index 58f65b92d..061b7948e 100644 --- a/library/agent/context/user.ts +++ b/library/agent/context/user.ts @@ -50,7 +50,6 @@ export function setUser(user: unknown) { if (context.executedMiddleware) { logWarningSetUserCalledAfterMiddleware(); - return; } context.user = validatedUser;