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

Allow different exception breakpoints from multiple debuggers to be shown at once #158355

Merged
merged 5 commits into from Nov 17, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
14 changes: 8 additions & 6 deletions src/vs/workbench/contrib/debug/browser/breakpointsView.ts
Expand Up @@ -63,8 +63,8 @@ function createCheckbox(disposables: IDisposable[]): HTMLInputElement {
}

const MAX_VISIBLE_BREAKPOINTS = 9;
export function getExpandedBodySize(model: IDebugModel, countLimit: number): number {
const length = model.getBreakpoints().length + model.getExceptionBreakpoints().length + model.getFunctionBreakpoints().length + model.getDataBreakpoints().length + model.getInstructionBreakpoints().length;
export function getExpandedBodySize(model: IDebugModel, sessionId: string | undefined, countLimit: number): number {
const length = model.getBreakpoints().length + model.getExceptionBreakpointsForSession(sessionId).length + model.getFunctionBreakpoints().length + model.getDataBreakpoints().length + model.getInstructionBreakpoints().length;
return Math.min(countLimit, length) * 22;
}
type BreakpointItem = IBreakpoint | IFunctionBreakpoint | IDataBreakpoint | IExceptionBreakpoint | IInstructionBreakpoint;
Expand Down Expand Up @@ -117,7 +117,7 @@ export class BreakpointsView extends ViewPane {
this.breakpointSupportsCondition = CONTEXT_BREAKPOINT_SUPPORTS_CONDITION.bindTo(contextKeyService);
this.breakpointInputFocused = CONTEXT_BREAKPOINT_INPUT_FOCUSED.bindTo(contextKeyService);
this._register(this.debugService.getModel().onDidChangeBreakpoints(() => this.onBreakpointsChange()));
this._register(this.debugService.getViewModel().onDidFocusSession(() => this.updateBreakpointsHint()));
this._register(this.debugService.getViewModel().onDidFocusSession(() => this.onBreakpointsChange()));
this._register(this.debugService.onDidChangeState(() => this.onStateChange()));
this.hintDelayer = this._register(new RunOnceScheduler(() => this.updateBreakpointsHint(true), 4000));
}
Expand Down Expand Up @@ -273,8 +273,9 @@ export class BreakpointsView extends ViewPane {
const containerModel = this.viewDescriptorService.getViewContainerModel(this.viewDescriptorService.getViewContainerByViewId(this.id)!)!;

// Adjust expanded body size
this.minimumBodySize = this.orientation === Orientation.VERTICAL ? getExpandedBodySize(this.debugService.getModel(), MAX_VISIBLE_BREAKPOINTS) : 170;
this.maximumBodySize = this.orientation === Orientation.VERTICAL && containerModel.visibleViewDescriptors.length > 1 ? getExpandedBodySize(this.debugService.getModel(), Number.POSITIVE_INFINITY) : Number.POSITIVE_INFINITY;
const sessionId = this.debugService.getViewModel().focusedSession?.getId();
this.minimumBodySize = this.orientation === Orientation.VERTICAL ? getExpandedBodySize(this.debugService.getModel(), sessionId, MAX_VISIBLE_BREAKPOINTS) : 170;
this.maximumBodySize = this.orientation === Orientation.VERTICAL && containerModel.visibleViewDescriptors.length > 1 ? getExpandedBodySize(this.debugService.getModel(), sessionId, Number.POSITIVE_INFINITY) : Number.POSITIVE_INFINITY;
}

private updateBreakpointsHint(delayed = false): void {
Expand Down Expand Up @@ -363,7 +364,8 @@ export class BreakpointsView extends ViewPane {

private get elements(): BreakpointItem[] {
const model = this.debugService.getModel();
const elements = (<ReadonlyArray<IEnablement>>model.getExceptionBreakpoints()).concat(model.getFunctionBreakpoints()).concat(model.getDataBreakpoints()).concat(model.getBreakpoints()).concat(model.getInstructionBreakpoints());
const sessionId = this.debugService.getViewModel().focusedSession?.getId();
const elements = (<ReadonlyArray<IEnablement>>model.getExceptionBreakpointsForSession(sessionId)).concat(model.getFunctionBreakpoints()).concat(model.getDataBreakpoints()).concat(model.getBreakpoints()).concat(model.getInstructionBreakpoints());

return elements as BreakpointItem[];
}
Expand Down
20 changes: 15 additions & 5 deletions src/vs/workbench/contrib/debug/browser/debugService.ts
Expand Up @@ -152,8 +152,12 @@ export class DebugService implements IDebugService {
this.disposables.add(this.viewModel.onDidFocusStackFrame(() => {
this.onStateChange();
}));
this.disposables.add(this.viewModel.onDidFocusSession(() => {
this.disposables.add(this.viewModel.onDidFocusSession((session: IDebugSession | undefined) => {
this.onStateChange();

if (session) {
this.setExceptionBreakpointFallbackSession(session.getId());
}
}));
this.disposables.add(Event.any(this.adapterManager.onDidRegisterDebugger, this.configurationManager.onDidSelectConfiguration)(() => {
const debugUxValue = (this.state !== State.Inactive || (this.configurationManager.getAllConfigurations().length > 0 && this.adapterManager.hasEnabledDebuggers())) ? 'default' : 'simple';
Expand Down Expand Up @@ -715,6 +719,8 @@ export class DebugService implements IDebugService {
}
}
}

this.model.removeExceptionBreakpointsForSession(session.getId());
}));
}

Expand Down Expand Up @@ -1046,8 +1052,13 @@ export class DebugService implements IDebugService {
await this.sendInstructionBreakpoints();
}

setExceptionBreakpoints(data: DebugProtocol.ExceptionBreakpointsFilter[]): void {
this.model.setExceptionBreakpoints(data);
setExceptionBreakpointFallbackSession(sessionId: string) {
this.model.setExceptionBreakpointFallbackSession(sessionId);
this.debugStorage.storeBreakpoints(this.model);
}

setExceptionBreakpointsForSession(session: IDebugSession, data: DebugProtocol.ExceptionBreakpointsFilter[]): void {
this.model.setExceptionBreakpointsForSession(session.getId(), data);
this.debugStorage.storeBreakpoints(this.model);
}

Expand Down Expand Up @@ -1121,9 +1132,8 @@ export class DebugService implements IDebugService {
}

private sendExceptionBreakpoints(session?: IDebugSession): Promise<void> {
const enabledExceptionBps = this.model.getExceptionBreakpoints().filter(exb => exb.enabled);

return sendToOneOrAllSessions(this.model, session, async s => {
const enabledExceptionBps = this.model.getExceptionBreakpointsForSession(s.getId()).filter(exb => exb.enabled);
if (s.capabilities.supportsConfigurationDoneRequest && (!s.capabilities.exceptionBreakpointFilters || s.capabilities.exceptionBreakpointFilters.length === 0)) {
// Only call `setExceptionBreakpoints` as specified in dap protocol #90001
return;
Expand Down
2 changes: 1 addition & 1 deletion src/vs/workbench/contrib/debug/browser/debugSession.ts
Expand Up @@ -312,7 +312,7 @@ export class DebugSession implements IDebugSession {

this.initialized = true;
this._onDidChangeState.fire();
this.debugService.setExceptionBreakpoints((this.raw && this.raw.capabilities.exceptionBreakpointFilters) || []);
this.debugService.setExceptionBreakpointsForSession(this, (this.raw && this.raw.capabilities.exceptionBreakpointFilters) || []);
} catch (err) {
this.initialized = true;
this._onDidChangeState.fire();
Expand Down
13 changes: 12 additions & 1 deletion src/vs/workbench/contrib/debug/common/debug.ts
Expand Up @@ -600,7 +600,18 @@ export interface IDebugModel extends ITreeElement {
areBreakpointsActivated(): boolean;
getFunctionBreakpoints(): ReadonlyArray<IFunctionBreakpoint>;
getDataBreakpoints(): ReadonlyArray<IDataBreakpoint>;

/**
* Returns list of all exception breakpoints.
*/
getExceptionBreakpoints(): ReadonlyArray<IExceptionBreakpoint>;

/**
* Returns list of exception breakpoints for the given session
* @param sessionId Session id. If falsy, returns the breakpoints from the last set fallback session.
*/
getExceptionBreakpointsForSession(sessionId?: string): ReadonlyArray<IExceptionBreakpoint>;

getInstructionBreakpoints(): ReadonlyArray<IInstructionBreakpoint>;
getWatchExpressions(): ReadonlyArray<IExpression & IEvaluate>;

Expand Down Expand Up @@ -1054,7 +1065,7 @@ export interface IDebugService {

setExceptionBreakpointCondition(breakpoint: IExceptionBreakpoint, condition: string | undefined): Promise<void>;

setExceptionBreakpoints(data: DebugProtocol.ExceptionBreakpointsFilter[]): void;
setExceptionBreakpointsForSession(session: IDebugSession, data: DebugProtocol.ExceptionBreakpointsFilter[]): void;

/**
* Sends all breakpoints to the passed session.
Expand Down
74 changes: 63 additions & 11 deletions src/vs/workbench/contrib/debug/common/debugModel.ts
Expand Up @@ -1056,14 +1056,17 @@ export class DataBreakpoint extends BaseBreakpoint implements IDataBreakpoint {

export class ExceptionBreakpoint extends BaseBreakpoint implements IExceptionBreakpoint {

private supportedSessions: Set<string> = new Set();

constructor(
public readonly filter: string,
public readonly label: string,
enabled: boolean,
public readonly supportsCondition: boolean,
condition: string | undefined,
public readonly description: string | undefined,
public readonly conditionDescription: string | undefined
public readonly conditionDescription: string | undefined,
private fallback: boolean = false
) {
super(enabled, undefined, condition, undefined, generateUuid());
}
Expand All @@ -1075,14 +1078,44 @@ export class ExceptionBreakpoint extends BaseBreakpoint implements IExceptionBre
result.enabled = this.enabled;
result.supportsCondition = this.supportsCondition;
result.condition = this.condition;
result.fallback = this.fallback;

return result;
}

setSupportedSession(sessionId: string, supported: boolean): void {
if (supported) {
this.supportedSessions.add(sessionId);
}
else {
this.supportedSessions.delete(sessionId);
}
}

/**
* Used to specify which breakpoints to show when no session is specified.
* Useful when no session is active and we want to show the exception breakpoints from the last session.
*/
setFallback(isFallback: boolean) {
this.fallback = isFallback;
roblourens marked this conversation as resolved.
Show resolved Hide resolved
}

get supported(): boolean {
return true;
}

/**
* Checks if the breakpoint is applicable for the specified session.
* If sessionId is undefined, returns true if this breakpoint is a fallback breakpoint.
*/
isSupportedSession(sessionId?: string): boolean {
return sessionId ? this.supportedSessions.has(sessionId) : this.fallback;
}

matches(filter: DebugProtocol.ExceptionBreakpointsFilter) {
return this.filter === filter.filter && this.label === filter.label && this.supportsCondition === !!filter.supportsCondition && this.conditionDescription === filter.conditionDescription && this.description === filter.description;
}

override toString(): string {
return this.label;
}
Expand Down Expand Up @@ -1340,26 +1373,45 @@ export class DebugModel implements IDebugModel {
return this.exceptionBreakpoints;
}

getExceptionBreakpointsForSession(sessionId?: string): IExceptionBreakpoint[] {
Copy link
Member Author

Choose a reason for hiding this comment

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

I've changes some usages of getExceptionBreakpoints with the new getExceptionBreakpointsForSession when appropriate.

The usage in logDebugSessionStart in debugTelemetry.ts, I'm not sure about what is the preferred one to use.

return this.exceptionBreakpoints.filter(ebp => ebp.isSupportedSession(sessionId));
}

getInstructionBreakpoints(): IInstructionBreakpoint[] {
return this.instructionBreakpoints;
}

setExceptionBreakpoints(data: DebugProtocol.ExceptionBreakpointsFilter[]): void {
setExceptionBreakpointsForSession(sessionId: string, data: DebugProtocol.ExceptionBreakpointsFilter[]): void {
if (data) {
if (this.exceptionBreakpoints.length === data.length && this.exceptionBreakpoints.every((exbp, i) =>
exbp.filter === data[i].filter && exbp.label === data[i].label && exbp.supportsCondition === data[i].supportsCondition && exbp.conditionDescription === data[i].conditionDescription && exbp.description === data[i].description)) {
// No change
return;
}
let didChangeBreakpoints = false;
data.forEach(d => {
let ebp = this.exceptionBreakpoints.filter((exbp) => exbp.matches(d)).pop();

if (!ebp) {
didChangeBreakpoints = true;
ebp = new ExceptionBreakpoint(d.filter, d.label, !!d.default, !!d.supportsCondition, undefined /* condition */, d.description, d.conditionDescription);
this.exceptionBreakpoints.push(ebp);
}

this.exceptionBreakpoints = data.map(d => {
const ebp = this.exceptionBreakpoints.filter(ebp => ebp.filter === d.filter).pop();
return new ExceptionBreakpoint(d.filter, d.label, ebp ? ebp.enabled : !!d.default, !!d.supportsCondition, ebp?.condition, d.description, d.conditionDescription);
ebp.setSupportedSession(sessionId, true);
});
this._onDidChangeBreakpoints.fire(undefined);

if (didChangeBreakpoints) {
this._onDidChangeBreakpoints.fire(undefined);
}
}
}

removeExceptionBreakpointsForSession(sessionId: string): void {
this.exceptionBreakpoints.forEach(ebp => ebp.setSupportedSession(sessionId, false));
}

// Set last focused session as fallback session.
// This is done to keep track of the exception breakpoints to show when no session is active.
setExceptionBreakpointFallbackSession(sessionId: string): void {
this.exceptionBreakpoints.forEach(ebp => ebp.setFallback(ebp.isSupportedSession(sessionId)));
}

setExceptionBreakpointCondition(exceptionBreakpoint: IExceptionBreakpoint, condition: string | undefined): void {
(exceptionBreakpoint as ExceptionBreakpoint).condition = condition;
this._onDidChangeBreakpoints.fire(undefined);
Expand Down
2 changes: 1 addition & 1 deletion src/vs/workbench/contrib/debug/common/debugStorage.ts
Expand Up @@ -59,7 +59,7 @@ export class DebugStorage {
let result: ExceptionBreakpoint[] | undefined;
try {
result = JSON.parse(this.storageService.get(DEBUG_EXCEPTION_BREAKPOINTS_KEY, StorageScope.WORKSPACE, '[]')).map((exBreakpoint: any) => {
return new ExceptionBreakpoint(exBreakpoint.filter, exBreakpoint.label, exBreakpoint.enabled, exBreakpoint.supportsCondition, exBreakpoint.condition, exBreakpoint.description, exBreakpoint.conditionDescription);
return new ExceptionBreakpoint(exBreakpoint.filter, exBreakpoint.label, exBreakpoint.enabled, exBreakpoint.supportsCondition, exBreakpoint.condition, exBreakpoint.description, exBreakpoint.conditionDescription, !!exBreakpoint.fallback);
});
} catch (e) { }

Expand Down
67 changes: 60 additions & 7 deletions src/vs/workbench/contrib/debug/test/browser/breakpoints.test.ts
Expand Up @@ -100,10 +100,10 @@ suite('Debug - Breakpoints', () => {
const modelUri1 = uri.file('/myfolder/my file first.js');
const modelUri2 = uri.file('/secondfolder/second/second file.js');
addBreakpointsAndCheckEvents(model, modelUri1, [{ lineNumber: 5, enabled: true }, { lineNumber: 10, enabled: false }]);
assert.strictEqual(getExpandedBodySize(model, 9), 44);
assert.strictEqual(getExpandedBodySize(model, undefined, 9), 44);

addBreakpointsAndCheckEvents(model, modelUri2, [{ lineNumber: 1, enabled: true }, { lineNumber: 2, enabled: true }, { lineNumber: 3, enabled: false }]);
assert.strictEqual(getExpandedBodySize(model, 9), 110);
assert.strictEqual(getExpandedBodySize(model, undefined, 9), 110);

assert.strictEqual(model.getBreakpoints().length, 5);
assert.strictEqual(model.getBreakpoints({ uri: modelUri1 }).length, 2);
Expand Down Expand Up @@ -137,7 +137,7 @@ suite('Debug - Breakpoints', () => {
assert.strictEqual(bp.enabled, true);

model.removeBreakpoints(model.getBreakpoints({ uri: modelUri1 }));
assert.strictEqual(getExpandedBodySize(model, 9), 66);
assert.strictEqual(getExpandedBodySize(model, undefined, 9), 66);

assert.strictEqual(model.getBreakpoints().length, 3);
});
Expand Down Expand Up @@ -213,22 +213,75 @@ suite('Debug - Breakpoints', () => {
test('exception breakpoints', () => {
let eventCount = 0;
model.onDidChangeBreakpoints(() => eventCount++);
model.setExceptionBreakpoints([{ filter: 'uncaught', label: 'UNCAUGHT', default: true }]);
model.setExceptionBreakpointsForSession("session-id-1", [{ filter: 'uncaught', label: 'UNCAUGHT', default: true }]);
assert.strictEqual(eventCount, 1);
let exceptionBreakpoints = model.getExceptionBreakpoints();
let exceptionBreakpoints = model.getExceptionBreakpointsForSession("session-id-1");
assert.strictEqual(exceptionBreakpoints.length, 1);
assert.strictEqual(exceptionBreakpoints[0].filter, 'uncaught');
assert.strictEqual(exceptionBreakpoints[0].enabled, true);

model.setExceptionBreakpoints([{ filter: 'uncaught', label: 'UNCAUGHT' }, { filter: 'caught', label: 'CAUGHT' }]);
model.setExceptionBreakpointsForSession("session-id-2", [{ filter: 'uncaught', label: 'UNCAUGHT' }, { filter: 'caught', label: 'CAUGHT' }]);
assert.strictEqual(eventCount, 2);
exceptionBreakpoints = model.getExceptionBreakpoints();
exceptionBreakpoints = model.getExceptionBreakpointsForSession("session-id-2");
assert.strictEqual(exceptionBreakpoints.length, 2);
assert.strictEqual(exceptionBreakpoints[0].filter, 'uncaught');
assert.strictEqual(exceptionBreakpoints[0].enabled, true);
assert.strictEqual(exceptionBreakpoints[1].filter, 'caught');
assert.strictEqual(exceptionBreakpoints[1].label, 'CAUGHT');
assert.strictEqual(exceptionBreakpoints[1].enabled, false);

model.setExceptionBreakpointsForSession("session-id-3", [{ filter: 'all', label: 'ALL' }]);
assert.strictEqual(eventCount, 3);
assert.strictEqual(model.getExceptionBreakpointsForSession("session-id-3").length, 1);
exceptionBreakpoints = model.getExceptionBreakpoints();
assert.strictEqual(exceptionBreakpoints[0].filter, 'uncaught');
assert.strictEqual(exceptionBreakpoints[0].enabled, true);
assert.strictEqual(exceptionBreakpoints[1].filter, 'caught');
assert.strictEqual(exceptionBreakpoints[1].label, 'CAUGHT');
assert.strictEqual(exceptionBreakpoints[1].enabled, false);
assert.strictEqual(exceptionBreakpoints[2].filter, 'all');
assert.strictEqual(exceptionBreakpoints[2].label, 'ALL');
});

test('exception breakpoints multiple sessions', () => {
let eventCount = 0;
model.onDidChangeBreakpoints(() => eventCount++);

model.setExceptionBreakpointsForSession("session-id-4", [{ filter: 'uncaught', label: 'UNCAUGHT', default: true }, { filter: 'caught', label: 'CAUGHT' }]);
model.setExceptionBreakpointFallbackSession("session-id-4");
assert.strictEqual(eventCount, 1);
let exceptionBreakpointsForSession = model.getExceptionBreakpointsForSession("session-id-4");
assert.strictEqual(exceptionBreakpointsForSession.length, 2);
assert.strictEqual(exceptionBreakpointsForSession[0].filter, 'uncaught');
assert.strictEqual(exceptionBreakpointsForSession[1].filter, 'caught');

model.setExceptionBreakpointsForSession("session-id-5", [{ filter: 'all', label: 'ALL' }, { filter: 'caught', label: 'CAUGHT' }]);
assert.strictEqual(eventCount, 2);
exceptionBreakpointsForSession = model.getExceptionBreakpointsForSession("session-id-5");
let exceptionBreakpointsForUndefined = model.getExceptionBreakpointsForSession(undefined);
assert.strictEqual(exceptionBreakpointsForSession.length, 2);
assert.strictEqual(exceptionBreakpointsForSession[0].filter, 'caught');
assert.strictEqual(exceptionBreakpointsForSession[1].filter, 'all');
assert.strictEqual(exceptionBreakpointsForUndefined.length, 2);
assert.strictEqual(exceptionBreakpointsForUndefined[0].filter, 'uncaught');
assert.strictEqual(exceptionBreakpointsForUndefined[1].filter, 'caught');

model.removeExceptionBreakpointsForSession("session-id-4");
assert.strictEqual(eventCount, 2);
exceptionBreakpointsForUndefined = model.getExceptionBreakpointsForSession(undefined);
assert.strictEqual(exceptionBreakpointsForUndefined.length, 2);
assert.strictEqual(exceptionBreakpointsForUndefined[0].filter, 'uncaught');
assert.strictEqual(exceptionBreakpointsForUndefined[1].filter, 'caught');

model.setExceptionBreakpointFallbackSession("session-id-5");
assert.strictEqual(eventCount, 2);
exceptionBreakpointsForUndefined = model.getExceptionBreakpointsForSession(undefined);
assert.strictEqual(exceptionBreakpointsForUndefined.length, 2);
assert.strictEqual(exceptionBreakpointsForUndefined[0].filter, 'caught');
assert.strictEqual(exceptionBreakpointsForUndefined[1].filter, 'all');

const exceptionBreakpoints = model.getExceptionBreakpoints();
assert.strictEqual(exceptionBreakpoints.length, 3);
});

test('instruction breakpoints', () => {
Expand Down
2 changes: 1 addition & 1 deletion src/vs/workbench/contrib/debug/test/common/mockDebug.ts
Expand Up @@ -93,7 +93,7 @@ export class MockDebugService implements IDebugService {
throw new Error('Method not implemented.');
}

setExceptionBreakpoints(data: DebugProtocol.ExceptionBreakpointsFilter[]): void {
setExceptionBreakpointsForSession(session: IDebugSession, data: DebugProtocol.ExceptionBreakpointsFilter[]): void {
throw new Error('Method not implemented.');
}

Expand Down