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

--inspect-brk runs immediately when the debugger attaches #6103

Closed
6 tasks done
leduyquang753 opened this issue Jul 12, 2024 · 8 comments · Fixed by #6110
Closed
6 tasks done

--inspect-brk runs immediately when the debugger attaches #6103

leduyquang753 opened this issue Jul 12, 2024 · 8 comments · Fixed by #6110

Comments

@leduyquang753
Copy link

Describe the bug

When a debugger like Chrome DevTools attaches to vitest --inspect-brk, the code is immediately and prematurely resumed execution.

It appears that the execution is resumed when Runtime.runIfWaitingForDebugger is called by the debugger. The execution is still paused when using the normal node --inspect-brk.

Reproduction

Very simple source files:

test.ts:

export function doubleUp(n: number): number {
	if (n > 0) n = n * 2;
	return n;
}

test.spec.ts:

import {expect, test} from "vitest";
import {doubleUp} from "./test";

test("Test", () => {
	expect(doubleUp(5)).toBe(10);
});

Run vitest --run --inspect-brk --no-file-parallelism and attach a debugger such as Chrome DevTools.

System Info

System:
  OS: Windows 11 10.0.22631
  CPU: (8) x64 Intel(R) Core(TM) i5-1035G1 CPU @ 1,00GHz
  Memory: 19,78 GB
Binaries:
  Node: 22.3.0 - C:\Program Files\NodeJS\node.EXE
  Yarn: 1.22.19 - ~\AppData\Roaming\npm\yarn.CMD
  npm: 10.8.1 - C:\Program Files\NodeJS\npm.CMD
Browsers:
  Edge: Chromium (126.0.2592.87)
  Internet Explorer: 11.0.22621.3527

vitest/2.0.2 win32-x64 node-v22.3.0

Used Package Manager

npm

Validations

@AriPerkkio
Copy link
Member

AriPerkkio commented Jul 12, 2024

I'm unable to reproduce this. Debugger stops immediately on top of the test.spec.ts when dev tools are attached. This was implemented in #5355.

image

Copy link

Hello @leduyquang753. Please provide a minimal reproduction using a GitHub repository or StackBlitz (you can also use examples). Issues marked with needs reproduction will be closed if they have no activity within 3 days.

@leduyquang753
Copy link
Author

leduyquang753 commented Jul 12, 2024

I'm unable to reproduce this. Debugger stops immediately on top of the test.spec.ts when dev tools are attached. This was implemented in #5355.

image

It might be dependent on the environment as that's macOS and I'm on Windows. Then what kind of "reproduction" do I have to do? Screen recording?

@AriPerkkio
Copy link
Member

The test case seems to be skipped on Windows too. Maybe there is indeed something broken.

test.skipIf(isWindows)('--inspect-brk stops at test file', async () => {

As most of us don't have access to Windows machines it would be amazing to get pull request with the fix for this. The related code is around here:

/**
* Enables debugging inside `worker_threads` and `child_process`.
* Should be called as early as possible when worker/process has been set up.
*/
export function setupInspect(ctx: ContextRPC) {

@leduyquang753
Copy link
Author

leduyquang753 commented Jul 12, 2024

Found the issue. It's that on Windows, file paths begin with the drive letter D: which when passed to URL on line 40 is mistaken as the URL's protocol. That, along with several other failing tests as I ran the test suite, shows me how much Vitest actually cares about Windows.

The bigger issue now is that when Vitest is run by inspect.test.ts, inspector.enabled becomes false for some oddball reason that I cannot debug with Visual studio code, nor were the various console.traces I sprinkled into the codebase hit. Would be great to have some assistance but I guess not.

@AriPerkkio
Copy link
Member

Found the issue. It's that on Windows, file paths begin with the drive letter D: which when passed to URL on line 40 is mistaken as the URL's protocol.

Awesome, thanks for looking into this! Could you try if this minimal repro works on Windows? If not, what changes are required for it to work? #5355 (comment)

That, along with several other failing tests as I ran the test suite [...]

We run all tests on CI on Windows too. The ones that are marked as skipIf(isWindow) are indeed skipped on Windows. Do you mean you removed some of those skipIf() and saw that the tests failed?

[...] shows me how much Vitest actually cares about Windows.

Do you have any suggestions how this could be improved? Always happy to extend support for all platforms. At least I thought our Windows support is very good. 🙌

for some oddball reason that I cannot debug with Visual studio code, nor were the various console.traces I sprinkled into the codebase hit. Would be great to have some assistance but I guess not.

I'm not sure I'm following here, but to me it sounds like you are seeing NodeJS issues with worker_threads debugging. It's simply not possible with some debuggers. Happy to help if you could write down the steps you did.

@leduyquang753
Copy link
Author

Could you try if this minimal repro works on Windows?

It does work and break properly.

We run all tests on CI on Windows too. The ones that are marked as skipIf(isWindow) are indeed skipped on Windows. Do you mean you removed some of those skipIf() and saw that the tests failed?

I had not done anything with the codebase at the time. I cloned the repository, set it up and ran the test suite right away. (test:all doesn't exist as a script, fix your docs.) Here are the failing ones: failing.txt.

I'm not sure I'm following here, but to me it sounds like you are seeing NodeJS issues with worker_threads debugging. It's simply not possible with some debuggers. Happy to help if you could write down the steps you did.

So the console.traces were after I gave up on using Visual studio code and I just manually added console.trace() lines into where I thought the code must have been going through like resolveConfig or even Logger.printBanner, then ran pnpm --filter vitest build. No trace was printed when I ran vitest after that, only console.logs I wrote into inspector.ts and worker.ts worked.

@AriPerkkio
Copy link
Member

Here are the failing ones

I'm also seeing these failures locally. Looks like all tests require CI=true flag that's passed by pnpm test:ci. We'll need to fix these internal tests.

It does work and break properly.

If I change the relative filepath to full absolute one, it doesn't work on Windows anymore. 🤔

Both of the versions below work on all platforms except Windows. After the change it works on Windows too, weird.

const filename = resolve("./target.mjs");
-const url = new URL(filename, import.meta.url).href; // Does not work on Windows
+const url = pathToFileURL(filename); // Works on all platforms

session.post("Debugger.setBreakpointByUrl", { url, lineNumber: 0 });

There's fix for the original issue on #6110. It also enables the test case on Windows CI which seems to pass fine now. Thanks for the root cause analysis!

If there's anything else that doesn't work on your final project on Windows, feel free to file bug reports for those. I'm still surprised to hear that this small bug on --inspect-brk gives the impression that Vitest doesn't care about Windows. Hopefully #6110 changes your view once next version is released. 🙌

@github-actions github-actions bot locked and limited conversation to collaborators Jul 28, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants