FazBrowse GitHub Viewer | Trending |
URL:
| Home
Tools: [Download Repo ZIP]   [Original HTTPS Page]

debugger: fix TypeError of list command in inspector repl by cola119 · Pull Request #42776 · nodejs/node · GitHub

/ node Public

debugger: fix TypeError of list command in inspector repl - #42776

Merged
aduh95 merged 5 commits into
nodejs:masterfrom
cola119:fix-cannot-read-props-of-null-error
May 22, 2022
ConversationCommits (5)ChecksFiles changed
Merged

debugger: fix TypeError of list command in inspector repl#42776
aduh95 merged 5 commits into
nodejs:masterfrom
cola119:fix-cannot-read-props-of-null-error

Conversation

cola119 commented Apr 18, 2022 •
edited by aduh95
Loading

Copy link
Copy Markdown
Member

Currently list command throws TypeError after the inspector is disconnected.
This PR fixed to throw ERR_DEBUGGER_ERROR instead of TypeError.

Fixes: #42775

nodejs-github-bot added debugger Issues and PRs related to the debugger subsystem. needs-ci PRs that need a full CI run. labels Apr 18, 2022
aduh95 reviewed Apr 18, 2022
View reviewed changes
Comment thread test/sequential/test-debugger-list.js Outdated
cola119 requested a review from aduh95 April 20, 2022 01:46

meixg commented Apr 20, 2022

Copy link
Copy Markdown
Member

There is a ci failure that seems related to the change : https://github.com/nodejs/node/runs/6070093179?check_suite_focus=true

cola119 force-pushed the fix-cannot-read-props-of-null-error branch from ae6defd to cd64887 Compare April 20, 2022 03:16

cola119 commented Apr 20, 2022

Copy link
Copy Markdown
Member Author

@meixg Thank you. I fixed. (but test/parallel/test-cluster-primary-kill.js failed 🥲

meixg added the request-ci Add this label to start a Jenkins CI on a PR. label Apr 20, 2022
github-actions Bot removed the request-ci Add this label to start a Jenkins CI on a PR. label Apr 20, 2022

nodejs-github-bot commented Apr 20, 2022

Copy link
Copy Markdown
Collaborator

CI: https://ci.nodejs.org/job/node-test-pull-request/43581/

aduh95 commented Apr 20, 2022 •
edited
Loading

Copy link
Copy Markdown
Contributor

Based on the CI failures, it looks like this is quite flaky on some configurations 🤔 Any idea what might be causing this?

cola119 commented Apr 20, 2022

Copy link
Copy Markdown
Member Author

@aduh95 It seems to be fixed by 2206bd0fb908491a21ae16f5db4beeee72a038bb

jasnell reviewed Apr 20, 2022
View reviewed changes
Comment thread test/sequential/test-debugger-list.js Outdated

jasnell Apr 20, 2022

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

Can this be refactored into an async function with awaits rather than a long then chain?

cola119 Apr 21, 2022

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

@jasnell done fea46820e4dc50e74340bc58e10c898bebfe7ed2

github-actions Bot mentioned this pull request Apr 21, 2022
Open
22 tasks
cola119 requested a review from jasnell April 21, 2022 02:58
aduh95 added the request-ci Add this label to start a Jenkins CI on a PR. label Apr 21, 2022
github-actions Bot removed the request-ci Add this label to start a Jenkins CI on a PR. label Apr 21, 2022

nodejs-github-bot commented Apr 21, 2022

Copy link
Copy Markdown
Collaborator

CI: https://ci.nodejs.org/job/node-test-pull-request/43610/

nodejs-github-bot commented Apr 21, 2022

Copy link
Copy Markdown
Collaborator

CI: https://ci.nodejs.org/job/node-test-pull-request/43621/

aduh95 commented Apr 21, 2022

Copy link
Copy Markdown
Contributor

There seems to be some related CI failures

node:internal/process/promises:288
            triggerUncaughtException(err, true /* fromPromise */);
            ^

AssertionError [ERR_ASSERTION]: The input did not match the regular expression /Uncaught Error \[ERR_DEBUGGER_ERROR\]: Requires execution to be paused/. Input:

'debug> '

    at /home/iojs/build/workspace/node-test-commit-linux/test/sequential/test-debugger-list.js:26:10
    at process.processTicksAndRejections (node:internal/process/task_queues:95:5) {
  generatedMessage: true,
  code: 'ERR_ASSERTION',
  actual: 'debug> ',
  expected: /Uncaught Error \[ERR_DEBUGGER_ERROR\]: Requires execution to be paused/,
  operator: 'match'
}

Node.js v19.0.0-pre

github-actions Bot mentioned this pull request Apr 22, 2022
Open
19 tasks
cola119 force-pushed the fix-cannot-read-props-of-null-error branch from fea4682 to 88f48e0 Compare April 22, 2022 03:59
aduh95 added the request-ci Add this label to start a Jenkins CI on a PR. label Apr 22, 2022
github-actions Bot removed the request-ci Add this label to start a Jenkins CI on a PR. label Apr 22, 2022

nodejs-github-bot commented Apr 22, 2022

Copy link
Copy Markdown
Collaborator

CI: https://ci.nodejs.org/job/node-test-pull-request/43633/

This was referenced May 20, 2022
Open
Open
Open

nodejs-github-bot commented May 22, 2022

Copy link
Copy Markdown
Collaborator

CI: https://ci.nodejs.org/job/node-test-pull-request/44116/

nodejs-github-bot commented May 22, 2022

Copy link
Copy Markdown
Collaborator

CI: https://ci.nodejs.org/job/node-test-pull-request/44122/

aduh95 added commit-queue Add this label to land a pull request using GitHub Actions. commit-queue-squash Add this label to instruct the Commit Queue to squash all the PR commits into the first one. and removed commit-queue Add this label to land a pull request using GitHub Actions. labels May 22, 2022
aduh95 merged commit cbb92b0 into nodejs:master May 22, 2022

aduh95 commented May 22, 2022

Copy link
Copy Markdown
Contributor

Landed in cbb92b0

This was referenced May 23, 2022
Open
Open
Open
Open
Open
Open
bengl pushed a commit that referenced this pull request May 30, 2022
debugger: throw a more useful error when the frame is missing
c76caad 
PR-URL: #42776
Fixes: #42775
Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
bengl mentioned this pull request May 31, 2022
Merged
juanarbol pushed a commit that referenced this pull request May 31, 2022
debugger: throw a more useful error when the frame is missing
1702a80 
PR-URL: #42776
Fixes: #42775
Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
juanarbol mentioned this pull request May 31, 2022
Closed

danielleadams commented Jun 27, 2022

Copy link
Copy Markdown
Contributor

@cola119 This breaks the test suite when trying to land in v16.x. Do you mind opening up a backport PR to v16.x-staging? (docs) Thank you.

cola119 mentioned this pull request Jun 27, 2022
Closed
cola119 added a commit to cola119/node that referenced this pull request Jun 28, 2022
debugger: throw a more useful error when the frame is missing
04b2f33 
PR-URL: nodejs#42776
Fixes: nodejs#42775
Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
targos pushed a commit that referenced this pull request Jul 18, 2022
debugger: throw a more useful error when the frame is missing
98a5670 
PR-URL: #42776
Fixes: #42775
Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
targos pushed a commit that referenced this pull request Jul 31, 2022
debugger: throw a more useful error when the frame is missing
f1c12aa 
PR-URL: #42776
Fixes: #42775
Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
targos mentioned this pull request Aug 3, 2022
Merged
guangwong pushed a commit to noslate-project/node that referenced this pull request Oct 10, 2022
debugger: throw a more useful error when the frame is missing
db4ef5c 
PR-URL: nodejs/node#42776
Fixes: nodejs/node#42775
Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters. Learn more about bidirectional Unicode characters
Show hidden characters
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

jasnell jasnell approved these changes
aduh95 aduh95 approved these changes
+1 more reviewer
austinkelleher austinkelleher left review comments
Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

author ready PRs that have at least one approval, no outstanding review comments, and a CI started. commit-queue-squash Add this label to instruct the Commit Queue to squash all the PR commits into the first one. debugger Issues and PRs related to the debugger subsystem. needs-ci PRs that need a full CI run.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Debugger's list command throws TypeError: Cannot read properties of null (reading 'list')

9 participants

Back | FazBrowse Home | New Git URL