Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

lib: fix emit warning for debuglog.time when disabled #54275

Merged
merged 5 commits into from
Aug 27, 2024
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
15 changes: 7 additions & 8 deletions lib/internal/util/debuglog.js
Original file line number Diff line number Diff line change
Expand Up @@ -133,6 +133,7 @@ function pad(value) {
const kNone = 1 << 0;
const kSkipLog = 1 << 1;
const kSkipTrace = 1 << 2;
const kShouldSkipAll = kSkipLog | kSkipTrace;

const kSecond = 1000;
const kMinute = 60 * kSecond;
Expand Down Expand Up @@ -377,8 +378,6 @@ function debugWithTimer(set, cb) {
let debugLogCategoryEnabled = false;
let timerFlags = kNone;

const skipAll = kSkipLog | kSkipTrace;

function ensureTimerFlagsAreUpdated() {
timerFlags &= ~kSkipTrace;

Expand All @@ -393,7 +392,7 @@ function debugWithTimer(set, cb) {
function internalStartTimer(logLabel, traceLabel) {
ensureTimerFlagsAreUpdated();

if (timerFlags === skipAll) {
if ((timerFlags & kShouldSkipAll) === kShouldSkipAll) {
return;
}

Expand All @@ -413,7 +412,7 @@ function debugWithTimer(set, cb) {
function internalEndTimer(logLabel, traceLabel) {
ensureTimerFlagsAreUpdated();

if (timerFlags === skipAll) {
if ((timerFlags & kShouldSkipAll) === kShouldSkipAll) {
return;
}

Expand All @@ -434,7 +433,7 @@ function debugWithTimer(set, cb) {
function internalLogTimer(logLabel, traceLabel, args) {
ensureTimerFlagsAreUpdated();

if (timerFlags === skipAll) {
if ((timerFlags & kShouldSkipAll) === kShouldSkipAll) {
return;
}

Expand Down Expand Up @@ -477,7 +476,7 @@ function debugWithTimer(set, cb) {
const startTimer = (logLabel, traceLabel) => {
init();

if (timerFlags !== skipAll)
if ((timerFlags & kShouldSkipAll) !== kShouldSkipAll)
internalStartTimer(logLabel, traceLabel);
};

Expand All @@ -487,7 +486,7 @@ function debugWithTimer(set, cb) {
const endTimer = (logLabel, traceLabel) => {
init();

if (timerFlags !== skipAll)
if ((timerFlags & kShouldSkipAll) !== kShouldSkipAll)
internalEndTimer(logLabel, traceLabel);
};

Expand All @@ -497,7 +496,7 @@ function debugWithTimer(set, cb) {
const logTimer = (logLabel, traceLabel, args) => {
init();

if (timerFlags !== skipAll)
if ((timerFlags & kShouldSkipAll) !== kShouldSkipAll)
internalLogTimer(logLabel, traceLabel, args);
};

Expand Down
4 changes: 4 additions & 0 deletions test/fixtures/GH-54265/dep1.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
// dep1.js
module.exports = function requireDep2() {
require("./dep2.js");
};
H4ad marked this conversation as resolved.
Show resolved Hide resolved
3 changes: 3 additions & 0 deletions test/fixtures/GH-54265/dep2.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
// dep2.js

// (empty)
H4ad marked this conversation as resolved.
Show resolved Hide resolved
10 changes: 10 additions & 0 deletions test/fixtures/GH-54265/index.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
// index.js
const Module = require("module");
const requireDep2 = require("./dep1.js");

const globalCache = Module._cache;
Module._cache = Object.create(null);
require("./require-hook.js");
Module._cache = globalCache;

requireDep2();
H4ad marked this conversation as resolved.
Show resolved Hide resolved
9 changes: 9 additions & 0 deletions test/fixtures/GH-54265/require-hook.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
// require-hook.js
const Module = require("module");
const requireDep2 = require("./dep1.js");

const originalJSLoader = Module._extensions[".js"];
Module._extensions[".js"] = function customJSLoader(module, filename) {
requireDep2();
return originalJSLoader(module, filename);
};
H4ad marked this conversation as resolved.
Show resolved Hide resolved
17 changes: 17 additions & 0 deletions test/parallel/test-module-print-timing.mjs
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ import { readFile } from 'node:fs/promises';
import { it } from 'node:test';
import tmpdir from '../common/tmpdir.js';
import { spawnSyncAndAssert } from '../common/child_process.js';
import fixtures from '../common/fixtures.js';

tmpdir.refresh();

Expand Down Expand Up @@ -153,3 +154,19 @@ it('should support enable tracing dynamically', async () => {
const vmTraces = outputFileJson.filter((trace) => trace.name === "require('vm')");
assert.strictEqual(vmTraces.length, 0);
});

it('should not print when is disabled and found duplicated labels (GH-54265)', () => {
const testFile = fixtures.path('GH-54265/index.js');

spawnSyncAndAssert(process.execPath, [
testFile,
], {
cwd: tmpdir.path,
env: {
...process.env,
},
}, {
stdout: '',
stderr: '',
});
});
Loading