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

Add support for conditional exception breakpoints #12445

Merged
merged 3 commits into from
Jun 8, 2023

Conversation

pisv
Copy link
Contributor

@pisv pisv commented Apr 21, 2023

What it does

Closes #12444 by adding support for conditional exception breakpoints to Theia:

How to test

Try to debug a simple Node.js program like this:

class MyError extends Error {
    constructor(message) {
        super(message);
        this.name = this.constructor.name;
    }
}

try {
    throw new Error();
} catch(e) {
    console.error(e);
}
try {
    throw new MyError();
} catch(e) {
    console.error(e);
}
throw new Error();

Enable both Caught Exceptions and Uncaught Exceptions in Breakpoints view. Try setting a condition like error.name == "MyError" using the context menu item Edit Condition... for either of the exception breakpoints. Verify that exception filtering works as expected while debugging. Also, verify that exception breakpoints' condition settings are properly restored upon workbench restart. Try removing the condition by editing it again and blanking out the input field.

Review checklist

Reminder for reviewers

@pisv
Copy link
Contributor Author

pisv commented Apr 21, 2023

The PR is ready for review.

@msujew msujew added dap debug adapter protocol debug issues that related to debug functionality labels Apr 22, 2023
@vince-fugnitto vince-fugnitto modified the milestone: 1.37.0 Apr 27, 2023
@tsmaeder tsmaeder self-requested a review May 30, 2023 12:27
Copy link
Contributor

@tsmaeder tsmaeder left a comment

Choose a reason for hiding this comment

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

Some suggestions

@@ -692,13 +692,26 @@ export class DebugSession implements CompositeTreeElement {
}

protected async sendExceptionBreakpoints(): Promise<void> {
const filters = [];
const args: DebugProtocol.SetExceptionBreakpointsArguments = {
Copy link
Contributor

Choose a reason for hiding this comment

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

Wouldn't this be simpler:

const filters= [];
const filterOptions= this.capabilities.supporteExceptionFilterOptions ? [] : undefined;

for (....) {
...
}
await this.sendRequest('setExceptionBreakpoints', { filters, filterOptions});

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Sure. Thanks for the suggestion!

@@ -31,13 +34,35 @@ export class DebugExceptionBreakpoint implements TreeElement {
}

render(): React.ReactNode {
return <div title={this.data.raw.label} className='theia-source-breakpoint'>
return <div title={this.data.raw.description || this.data.raw.label} className='theia-source-breakpoint'>
Copy link
Contributor

Choose a reason for hiding this comment

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

Drive-by fix?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

That's right.

@tsmaeder
Copy link
Contributor

tsmaeder commented Jun 7, 2023

The behaviour seems fine. Would you mind fixing the conflicts?

@pisv
Copy link
Contributor Author

pisv commented Jun 7, 2023

Would you mind fixing the conflicts?

Done. Thank you for review.

@tsmaeder tsmaeder merged commit 44316b3 into eclipse-theia:master Jun 8, 2023
@pisv pisv deleted the GH-12444 branch June 8, 2023 09:53
@vince-fugnitto vince-fugnitto added this to the 1.39.0 milestone Jun 29, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
dap debug adapter protocol debug issues that related to debug functionality
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

Add support for conditional exception breakpoints
4 participants