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

feat(vscode-extension) error reporting, qol #15261

Merged
merged 94 commits into from
Nov 22, 2024

Conversation

alii
Copy link
Collaborator

@alii alii commented Nov 19, 2024

What does this PR do?

A few goodies implemented into the VSCode extension.

How did you verify your code works?

Ran extension and tested each feature

Notes:

  • Async stack traces are NOT supported. This is because JSC does not actually record them unless the debugger is open. The debugger is big overhead, so it's not open in this case.
  • Ideally we would show the function name of .relatedInformation for each diagnostic. Right now we just show the error message again. This isn't amazing...
  • Disabling this right now would be done by setting the BUN_INSPECT_NOTIFY variable to ''. A use case for disabling would be like when running a benchmark file (where this would actually have a very very slight performance overhead)
  • The socket framer on the extension side could be rewritten

@alii alii force-pushed the feat/vscode-diagnostics branch from a393fad to 93b52be Compare November 19, 2024 23:23
@alii alii marked this pull request as ready for review November 19, 2024 23:30
@alii alii changed the title Feat/vscode diagnostics feat(vscode-extension) error reporting, qol Nov 19, 2024
@alii alii requested a review from Jarred-Sumner November 19, 2024 23:40
@alii alii marked this pull request as draft November 20, 2024 18:34
}

for (size_t i = 0; i < exception.stack.frames_len; i++) {
ZigStackFrame* frame = &exception.stack.frames_ptr[i];
Copy link
Member

Choose a reason for hiding this comment

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

In the future, I think we can get the function name from frame here

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Interesting, we'd need to add this to the inspector event. Probably can't guarantee that we'll always have it (@Jarred-Sumner ?)

@alii alii marked this pull request as ready for review November 21, 2024 23:47
@alii
Copy link
Collaborator Author

alii commented Nov 22, 2024

cc @Jarred-Sumner @dylan-conway
CleanShot 2024-11-21 at 17 28 38@2x
Could this actually be relevant? Or expected that some/certain tests are failing here

@dylan-conway
Copy link
Member

Yup, this is a new test failure, looking into it. I believe the cause is from https://github.com/oven-sh/bun/pull/15261/files#diff-36302f74ed5420c7bee6a5460115be29fd82720720381c0d2c8bb7c934e69591L382

@alii
Copy link
Collaborator Author

alii commented Nov 22, 2024

This code (or at least something similar iirc) was removed previously, I think because it was printing errors twice. Did you test this locally and if so does it seem to be working? I can try it out as well to test

@Jarred-Sumner Jarred-Sumner merged commit 4117af6 into oven-sh:main Nov 22, 2024
60 of 64 checks passed
@alii alii deleted the feat/vscode-diagnostics branch November 22, 2024 15:05
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants