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

src: fix syntax error not showing in stderr with node --inspect-brk #42486

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

cola119
Copy link
Member

@cola119 cola119 commented Mar 27, 2022

Fixed #41792

When an uncaught exception occurs with inspector enabled, node::errors::TriggerUncaughtException will let an inspector agent send the error report.

report_to_inspector();

After reporting errors, the inspector agent waits for the client to disconnect. But this means the agent stops the following process and shows nothing in stderr.

Ideally, AtExit hook should be responsible for disconnection of inspector and the error reporters should send error information only. An inspector agent has no reason to wait for disconnection since the disconnection process is already registered to AtExit hook.

node/src/inspector_agent.cc

Lines 711 to 716 in 6e54851

AtExit(parent_env_, [](void* env) {
Agent* agent = static_cast<Environment*>(env)->inspector_agent();
if (agent->IsActive()) {
agent->WaitForDisconnect();
}
}, parent_env_);

@nodejs-github-bot nodejs-github-bot added c++ Issues and PRs that require attention from people who are familiar with C++. inspector Issues and PRs related to the V8 inspector protocol needs-ci PRs that need a full CI run. labels Mar 27, 2022
@RaisinTen RaisinTen requested a review from jkrems March 27, 2022 07:16
@nodejs-github-bot
Copy link
Collaborator

@jkrems
Copy link
Contributor

jkrems commented Mar 28, 2022

I'm not sure if I fully understand the implications of removing WaitForDisconnect here. It seems okay but maybe somebody from @nodejs/v8 or @nodejs/v8-inspector has the full picture?

@nodejs-github-bot
Copy link
Collaborator

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
c++ Issues and PRs that require attention from people who are familiar with C++. inspector Issues and PRs related to the V8 inspector protocol needs-ci PRs that need a full CI run.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Syntax error not showing in stderr with node --inspect-brk in node 12+
4 participants