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

Exception breakpoints #218

Merged
merged 7 commits into from
Nov 17, 2017
Merged

Conversation

DatGuyJonathan
Copy link
Contributor

What does this PR do?

exceptionbreakpoint

This change allows a debugging user to add/remove exception breakpoints for System and user-defined exception types. The list of exception types are acquired from Apex language server. Configuring an exception breakpoint is done through the command palette.

Typical UX Flow

  1. Open SFDX project in VS Code with Salesforce extensions installed
  2. Start Apex Debugger session
  3. Open command palette
  4. Select Apex Debug: Configure Exceptions
  5. Select an exception type
  6. Select whether to always break (add breakpoint) or never break (remove breakpoint)
  7. Wait until a log line in Debug Console verifies the breakpoint has been configured
  8. Trigger the Apex exception

Notes

  • Exception types in the command palette are listed alphabetically.
  • Enabled exception breakpoints will be shown at the top of the list of exception types in the command palette. They will have a description that says "Always break" with a built-in VS Code icon from https://octicons.github.com/icon/stop/.
  • Apex Debug: Configure Exceptions command is also available without an active debugger session. If the user tries to add an exception breakpoint without an active session, we automatically mark it as enabled and save it to be added when we detect a new debugger session.
  • packages/salesforcedx-vscode-apex/out/apex-jorje-lsp.jar is signed.

What issues does this PR fix or reference?

@W-4179085@

@DatGuyJonathan DatGuyJonathan requested a review from guw November 17, 2017 01:21
@@ -977,10 +987,56 @@ export class ApexDebug extends LoggingDebugSession {
this.myRequestService.connectionTimeoutMs =
workspaceSettings.connectionTimeoutMs;
break;
case EXCEPTION_BREAKPOINT_REQUEST:
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This is a request from the extension to the adapter for configuring an exception breakpoint.

}
}
break;
case LIST_EXCEPTION_BREAKPOINTS_REQUEST:
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This is a request from the extension to the adapter for a list of enabled exception breakpoints.

.withArg('--json')
.build(),
{ cwd: projectPath, env: RequestService.getEnvVars() }
).execute();
Copy link
Contributor Author

Choose a reason for hiding this comment

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

CLI command to create an ApexDebuggerBreakpoint object with for an exception type.

info: ExceptionBreakpointInfo
): Promise<void> {
const knownBreakpointId = this.exceptionBreakpointCache.get(info.typeref);
if (knownBreakpointId && info.breakMode === 'never') {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Breakpoint service will delete a breakpoint if it exists in the cache and the user choses Never break for the exception type.

if (knownBreakpointId && info.breakMode === 'never') {
await this.deleteBreakpoint(projectPath, knownBreakpointId);
this.exceptionBreakpointCache.delete(info.typeref);
} else if (!knownBreakpointId && info.breakMode === 'always') {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Breakpoint service will add a breakpoint if it doesn't exist in the cache and the user choses Always break for the exception type.

Copy link
Contributor

Choose a reason for hiding this comment

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

Is this logic a result of limitations with the CLI and updating the IsEnabled attribute of an exception breakpoint? It seems that VS Code is becoming the system or record then. On the other hand, we are dealing with "short" lived debugging sessions anyway. Thus, a sync issue may just be worked around by killing and starting a new debug session.

"You can't modify Apex classes or triggers during an Apex Debugger session. Save your changes after you're done debugging."
"You can't modify Apex classes or triggers during an Apex Debugger session. Save your changes after you're done debugging.",
created_exception_breakpoint_text: 'Created exception breakpoint for %s.',
removed_exception_breakpoint_text: 'Removed exception breakpoint for %s.'
Copy link
Contributor Author

Choose a reason for hiding this comment

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

These messages appear in the Debug Console when there's an active debugger session and an exception breakpoint is added/removed.

@@ -14,5 +14,6 @@
"Produce diagnostic output. Instead of setting this to true you can list one or more selectors separated with commas. The 'all' selector enables very detailed output. Other selectors are 'protocol', 'variables', 'launch'",
"configuration_title": "Salesforce Apex Debugger Configuration",
"connection_timeout_ms_description":
"Connection timeout for Apex Debugger API requests (in milliseconds)."
"Connection timeout for Apex Debugger API requests (in milliseconds).",
"exception_breakpoint_command_text": "Apex Debug: Configure Exceptions"
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Label for the command.

'salesforce.salesforcedx-vscode-apex'
);
if (sfdxApex && sfdxApex.exports) {
const exceptionBreakpointInfos: ExceptionBreakpointItem[] = await sfdxApex.exports.getExceptionBreakpointInfo();
Copy link
Contributor Author

Choose a reason for hiding this comment

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

The extension gets a list of System and user-defined exception types from Apex language server.

const args: SetExceptionBreakpointsArguments = {
exceptionInfo: breakpoint
};
session.customRequest(EXCEPTION_BREAKPOINT_REQUEST, args);
Copy link
Contributor Author

Choose a reason for hiding this comment

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

When a debugger session starts, add all exception breakpoints that were selected while without a debugger session.

cachedExceptionBreakpoints.keys()
);
}
const processedBreakpointInfos = mergeExceptionBreakpointInfos(
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Create the quick pick list of exception types. Enabled breakpoints are listed first.

for (let i = breakpointInfos.length - 1; i >= 0; i--) {
if (enabledBreakpointTyperefs.indexOf(breakpointInfos[i].typeref) >= 0) {
breakpointInfos[i].breakMode = EXCEPTION_BREAKPOINT_BREAK_MODE_ALWAYS;
breakpointInfos[i].description = `$(stop) ${nls.localize(
Copy link
Contributor Author

Choose a reason for hiding this comment

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

The description will show an icon from https://octicons.github.com/icon/stop/ and Always break if it's an enabled breakpoint.

@@ -31,5 +35,13 @@ async function getLineBreakpointInfo(): Promise<{}> {
return Promise.resolve(response);
}

async function getExceptionBreakpointInfo(): Promise<{}> {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Exposed API in salesforcedx-vscode-apex to retrieve a list of System and user-defined exception types.

@DatGuyJonathan
Copy link
Contributor Author

DatGuyJonathan commented Nov 17, 2017

@guw , this is a huge changelist. Take your time reviewing it. There's no scheduled release next week, so the latest we want to get this merged in is November 27th EOD. The release will be November 30th.

@ruthemmanuelle , there's some UX text you can review in files named i18n.ts. Also a heads up to set aside some time to update https://github.com/forcedotcom/salesforcedx-vscode/tree/develop/packages/salesforcedx-vscode-apex-debugger.

*
* If ommitted, we will assume _message.
*/
export const messages = {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

These are labels shown in the command palette when selecting an exception. See .gif for an example.

@codecov
Copy link

codecov bot commented Nov 17, 2017

Codecov Report

Merging #218 into develop will increase coverage by 0.61%.
The diff coverage is 44.3%.

Impacted file tree graph

@@             Coverage Diff             @@
##           develop     #218      +/-   ##
===========================================
+ Coverage    80.31%   80.93%   +0.61%     
===========================================
  Files          111      113       +2     
  Lines         4527     4637     +110     
  Branches       745      770      +25     
===========================================
+ Hits          3636     3753     +117     
+ Misses         735      725      -10     
- Partials       156      159       +3
Impacted Files Coverage Δ
...pex-debugger/test/unit/adapter/apexDebugForTest.ts 100% <ø> (ø) ⬆️
...es/salesforcedx-apex-debugger/src/messages/i18n.ts 100% <ø> (ø) ⬆️
packages/salesforcedx-apex-debugger/src/index.ts 100% <ø> (ø) ⬆️
...ges/salesforcedx-vscode-apex-debugger/src/index.ts 21.38% <10.71%> (-2.26%) ⬇️
...sforcedx-vscode-apex-debugger/src/messages/i18n.ts 100% <100%> (ø)
packages/salesforcedx-vscode-apex/src/constants.ts 100% <100%> (ø) ⬆️
...ckages/salesforcedx-apex-debugger/src/constants.ts 100% <100%> (ø) ⬆️
packages/salesforcedx-vscode-apex/src/index.ts 47.36% <16.66%> (-16.92%) ⬇️
...forcedx-vscode-apex-debugger/src/messages/index.ts 86.66% <86.66%> (ø)
...orcedx-apex-debugger/src/core/breakpointService.ts 91.2% <86.66%> (-1.75%) ⬇️
... and 8 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 4cb35f0...fd51774. Read the comment docs.

Copy link
Contributor

@guw guw left a comment

Choose a reason for hiding this comment

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

LGTM

Copy link
Contributor

@ruthemmanuelle ruthemmanuelle left a comment

Choose a reason for hiding this comment

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

The messages look fine. I didn't review anything else.

@DatGuyJonathan DatGuyJonathan merged commit 54b1279 into develop Nov 17, 2017
@DatGuyJonathan DatGuyJonathan deleted the jwidjaja/exceptionBreakpoints branch November 17, 2017 22:57
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.

3 participants