nodejs / node

Node.js JavaScript runtime ✨🐢🚀✨
https://nodejs.org
Other
106.57k stars 29.05k forks source link

Option to hide "Debugger attached" console log on connected debugger #34799

Open sebmck opened 4 years ago

sebmck commented 4 years ago

Is your feature request related to a problem? Please describe.

Our project Rome has a test runner. We spawn worker_threads to execute tests. We attach a debugger to each of them so we can collect stacktraces when tests timeout and have a blocked event loop. Right now, Node always outputs a "Debugger attached" message to stderr src/inspector_io.cc. This leads to console spam whenever a test worker is started. That isn't ideal for an implementation detail and looks like a bug to a user.

Describe the solution you'd like

A way to disable the "Debugger attached" message, possibly via an environment variable or CLI flag.

Describe alternatives you've considered

Previously we used forked child processes and so had a dedicated stderr where we could intercept and hide it. Unfortunately, it seems like there's no way to do that specifically with everything in the same process and that the log never happens in JS.

aduh95 commented 4 years ago

FWIW, it's already possible to detach worker's standard IO:

$  node -e "new (require('worker_threads').Worker)('console.error(4)', {eval:true})"
4
$  node -e "new (require('worker_threads').Worker)('console.error(4)', {eval:true, stderr:true})"
$
devsnek commented 4 years ago

@sebmck are you starting the inspector via the require('inspector') api? I find it very annoying that starting the inspector programmatically prints stuff out.

sebmck commented 4 years ago

Sorry, I should have provided a repro snippet. stderr: true, stdout: true has no effect on how the "Debugger attached" log is written.

I'm using inspector.open(); in a worker thread, passing inspector.url() back to the main thread, and connecting to it with a websocket.

$ node --inspect-publish-uid=http worker.js
Debugger attached.
const {Worker} = require('worker_threads');
const crypto = require('crypto');
const http = require('http');
const net = require('net');

const worker = new Worker(
  "require('inspector').open(8888); require('worker_threads').parentPort.postMessage(require('inspector').url()); setInterval(() => {}, 1000);",
  {eval: true, stdout: true, stderr: true, stdin: true},
);

worker.on('message', (url) => {
  http.request({
    hostname: "127.0.0.1",
    port: 8888,
    path: "/" + url.split("/").pop(),
    method: "GET",
    headers: {
      Connection: "Upgrade",
      Upgrade: "websocket",
      "Sec-WebSocket-Key": "foobar",
      "Sec-WebSocket-Version": "13",
    },
  }).end();
});
devsnek commented 4 years ago

Not sure whether to call this a bug or a feature request, but it should definitely be fixed.

nathanblair commented 4 years ago

This would make working with vs code much nicer as well. I think there's probably some magic that could be done to redirect the stderr stream of this information to some null fd or something but a built-in flag would be 😍

bnoordhuis commented 4 years ago

I'd be okay with removing that specific message, I don't think it adds much value. Or are there are third-party tools using it as an eye catcher?

Here's a diff, feel free to steal and pull request to get the conversation going:

diff --git a/src/inspector_io.cc b/src/inspector_io.cc
index d3bd191121..9e5c9d281b 100644
--- a/src/inspector_io.cc
+++ b/src/inspector_io.cc
@@ -341,10 +341,7 @@ void InspectorIoDelegate::StartSession(int session_id,
   auto session = main_thread_->Connect(
       std::unique_ptr<InspectorSessionDelegate>(
           new IoSessionDelegate(request_queue_->handle(), session_id)), true);
-  if (session) {
-    sessions_[session_id] = std::move(session);
-    fprintf(stderr, "Debugger attached.\n");
-  }
+  if (session) sessions_[session_id] = std::move(session);
 }

 void InspectorIoDelegate::MessageReceived(int session_id,
justingrant commented 3 years ago

I just ran into this problem while trying to set up VSCode debugging of tests run by the the Test262 test harness used to test JS engines like V8. This test harness treats any console output as a test failure.

So it'd be great to have some way to hide these messages so that these tests won't have false-negative failures when run under the debugger.

justingrant commented 3 years ago

Note that I'd want to remove both the attaching ("Debugger attached.\n") and detaching ("Waiting for the debugger to disconnect...\n") messages.