Skip to content

fix: guard global.metrics in singleFlightMiddleware - #670

Open
mateussaggin wants to merge 2 commits into
masterfrom
fix/guard-global-metrics-in-inflight
Open

fix: guard global.metrics in singleFlightMiddleware#670
mateussaggin wants to merge 2 commits into
masterfrom
fix/guard-global-metrics-in-inflight

Conversation

@mateussaggin

Copy link
Copy Markdown

What

Guards the bare metrics global in singleFlightMiddleware, and fixes the size reported by its flush metric.

Why

Until v7, global.metrics = new MetricsAccumulator() was a module-load side effect of service/Runtime.ts, so requiring the package initialized it. In v7 that moved inside startApp() (service/index.ts), which only runs on the IO service runtime.

singleFlightMiddleware still references metrics as a bare global. Any standalone consumer of HttpClient — one that never boots the service runtime — therefore crashes on the first request that sets an inflightKey:

ReferenceError: metrics is not defined
    at singleFlightMiddleware (lib/HttpClient/middlewares/inflight.js:16:9)

This is what broke the toolbelt CLI in v4.4.2 when it bumped to @vtex/api@7.4.0 (see vtex/toolbelt#1276 and vtex/toolbelt#1277). The toolbelt PR fixes the CLI by initializing the global itself; this PR makes the library not depend on a global only its own service runtime creates, so the whole class of bug can't recur for other consumers.

Note metricsMiddleware already takes metrics as an explicit (optional) parameter and guards it — inflight.ts is the one place still reaching for the bare global.

Also

-      size: inflight.entries.length,
+      size: inflight.size,

entries is a Map method, so inflight.entries.length evaluated to the function's arity (0) rather than the number of inflight requests. The node-vtex-api-inflight-map-size metric has been reporting a constant zero.

Behavior

Inside the service runtime nothing changes: global.metrics is defined, the flush metric is registered as before (now with a correct size). Outside it, the middleware works and simply skips the legacy metric registration.

@sonar-workflows

Copy link
Copy Markdown

Failed Quality Gate failed

  • 315 New Issues (is greater than 0)
  • 30.10% Coverage on New Code (is less than 75.00%)
  • 0.00% Security Hotspots Reviewed on New Code (is less than 100.00%)
  • D Reliability Rating on New Code (is worse than A)

Project ID: node-vtex-api

View in SonarQube

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants