FormidableLabs / envy

Node.js Telemetry & Network Viewer
https://envy-webui.vercel.app
MIT License
107 stars 0 forks source link

Fix nextjs duplicate connections causing duplicate traces to be sent #201

Closed kgpax closed 8 months ago

kgpax commented 8 months ago

What does this PR do

What was the cause of the issue?

There is code in /packages/nextjs/src/tracing.ts which invokes the @envyjs/node enableTracing function:

import { enableTracing as nodeTracing } from '@envyjs/node';

let initialized = false;

// ...

if (!initialized) {
  initialized = true;
  return nodeTracing({
    ...nextjsOptions,
    plugins: [...(options.plugins || []), Routes],
  });
}

This code looks like it should prevent the call to nodeTracing happening more than once; however I could put a console.log inside that condition and observe that on some occasions (such as reloading the web page of the app being traced, or by making a code change triggering HMR) I would see this being logged twice, therefore calling nodeTracing twice.

Once this happened, requests made by API routes started to appear duplicated in Envy.

What did I try?

1. Mutex locking

First I tried adding locking to the code to prevent the code from executing when it was locked. I used the async-mutex package to do something like this:

import { enableTracing as nodeTracing } from '@envyjs/node';
import { Mutex } from 'async-mutex';

const mutex = new Mutex();

let initialized = false;

// ...

mutex
  .acquire()
  .then(release => {
    if (!initialized) {
      initialized = true;
      return nodeTracing({
        ...nextjsOptions,
        plugins: [...(options.plugins || []), Routes],
      });
    }
    release();
  });

👎 However, this didn't seem to make a difference - it was still possible for this registration to happen twice.

2. Debouncing

Then I tried debouncing the call, such that duplicate attempts to make the connect would cancel the previous attempt(s) if fast enough...

import { enableTracing as nodeTracing } from '@envyjs/node';
import { debounce } from '@envyjs/core'; // I created the debounce util here

let initialized = false;

const debouncedInitialize = debounce((nextjsOptions: NextjsTracingOptions) => {
  if (!initialized) {
    initialized = true;
    return nodeTracing(nextjsOptions);
  }
}, 300); // debounce calls made within 300ms of each other

// ...

debouncedInitialize({
  ...nextjsOptions,
  plugins: [...(options.plugins || []), Routes],
});

👎 Still, this didn't prevent the issue.

3. Node global flag

Based on the above, I concluded that perhaps the problem is that this packages/nextjs/src/tracing module is being loaded as two separate instances, meaning that the initialized flag (or the mutex, or the debounced function) scoped to that module is only useful for code re-using that instance, and not for code spawning another instance. What we needed was a more guaranteed global scope for this flag. Therefore I changed to use the node global object to store the flag indicating that we have initialized:

import { enableTracing as nodeTracing } from '@envyjs/node';

type GlobalWithFlag = { __nextjsTracingInitialized: boolean };
const globalWithFlag: GlobalWithFlag = global as unknown as GlobalWithFlag;

// ...

if (!globalWithFlag.__nextjsTracingInitialized) {
  globalWithFlag.__nextjsTracingInitialized = true;
  return nodeTracing({
    ...nextjsOptions,
    plugins: [...(options.plugins || []), Routes],
  });
}

🎉 This seems to work, and I've tested it on the project that I originally observed the bug on by monkey-patching this fix.


Fixes #200

changeset-bot[bot] commented 8 months ago

🦋 Changeset detected

Latest commit: 4d9746b20e3e34b4865251d808b8a0beb512c485

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 6 packages | Name | Type | | -------------- | ----- | | @envyjs/nextjs | Patch | | @envyjs/apollo | Patch | | @envyjs/core | Patch | | @envyjs/node | Patch | | @envyjs/web | Patch | | @envyjs/webui | Patch |

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR