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

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

Open
sebmck opened this issue Aug 16, 2020 · 8 comments
Open

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

sebmck opened this issue Aug 16, 2020 · 8 comments
Labels
inspector Issues and PRs related to the V8 inspector protocol

Comments

@sebmck
Copy link

sebmck commented Aug 16, 2020

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
Copy link
Contributor

aduh95 commented Aug 16, 2020

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 devsnek added the inspector Issues and PRs related to the V8 inspector protocol label Aug 16, 2020
@devsnek
Copy link
Member

devsnek commented Aug 16, 2020

@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
Copy link
Author

sebmck commented Aug 16, 2020

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
Copy link
Member

devsnek commented Aug 16, 2020

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

@nathanblair
Copy link
Contributor

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
Copy link
Member

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
Copy link

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
Copy link

justingrant commented Sep 9, 2021

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
inspector Issues and PRs related to the V8 inspector protocol
Projects
None yet
Development

No branches or pull requests

6 participants