fix: push error handling (#5392)

* fix: push error handling

* chore: response.name upon conflicts

* chore: ahora si

---------

Co-authored-by: Daphne Yang <139700604+daphne-sfdc@users.noreply.github.com>

packages/salesforcedx-test-utils-vscode/src/orgUtils.ts

@@ -96,7 +96,7 @@ export const pushSource = async (
  ).execute();
  const cmdOutput = new CommandOutput();
  const result = await cmdOutput.getCmdResult(execution);
- const source = JSON.parse(result).result.files;
+ const source = JSON.parse(result).files;
  return Promise.resolve(source);
 };
 

packages/salesforcedx-utils-vscode/src/cli/parsers/projectDeployStartResultParser.ts

@@ -7,7 +7,7 @@
 
 import { extractJsonObject } from '../../helpers';
 
-export const CONFLICT_ERROR_NAME = 'sourceConflictDetected';
+export const CONFLICT_ERROR_NAME = 'SourceConflictError';
 
 export interface ProjectDeployStartResult {
  columnNumber?: string;
@@ -22,9 +22,8 @@ export interface ProjectDeployStartResult {
 export interface ProjectDeployStartErrorResponse {
  message: string;
  name: string;
- data: ProjectDeployStartResult[];
- stack: string;
  status: number;
+ files: ProjectDeployStartResult[];
  warnings: any[];
 }
 
@@ -50,7 +49,12 @@ export class ProjectDeployStartResultParser {
 
  public getErrors(): ProjectDeployStartErrorResponse | undefined {
  if (this.response.status === 1) {
- return this.response as ProjectDeployStartErrorResponse;
+ return {
+ message: this.response.message ?? 'Push failed. ',
+ name: this.response.name ?? 'DeployFailed',
+ status: this.response.status,
+ files: this.response.data ?? this.response.result.files
+ } as ProjectDeployStartErrorResponse;
  }
  }
 

packages/salesforcedx-vscode-core/src/commands/projectDeployStart.ts

@@ -21,8 +21,8 @@ import {
 import * as vscode from 'vscode';
 import { channelService } from '../channels';
 import { PersistentStorageService } from '../conflict';
-import { FORCE_SOURCE_PUSH_LOG_NAME } from '../constants';
-import { handleDiagnosticErrors } from '../diagnostics';
+import { PROJECT_DEPLOY_START_LOG_NAME } from '../constants';
+import { handlePushDiagnosticErrors } from '../diagnostics';
 import { nls } from '../messages';
 import { telemetryService } from '../telemetry';
 import { DeployRetrieveExecutor } from './baseDeployRetrieve';
@@ -124,7 +124,7 @@ export class ProjectDeployStartExecutor extends SfdxCommandletExecutor<{}> {
  cancellationTokenSource: vscode.CancellationTokenSource
  /* eslint-enable @typescript-eslint/no-unused-vars */
  ): Promise<void> {
- if (execution.command.logName === FORCE_SOURCE_PUSH_LOG_NAME) {
+ if (execution.command.logName === PROJECT_DEPLOY_START_LOG_NAME) {
  const pushResult = this.parseOutput(stdOut);
  if (exitCode === 0) {
  this.updateCache(pushResult);
@@ -140,7 +140,7 @@ export class ProjectDeployStartExecutor extends SfdxCommandletExecutor<{}> {
  const errors = pushParser.getErrors();
  if (errors && !pushParser.hasConflicts()) {
  channelService.showChannelOutput();
- handleDiagnosticErrors(
+ handlePushDiagnosticErrors(
  errors,
  workspacePath,
  execFilePathOrPaths,
@@ -191,7 +191,7 @@ export class ProjectDeployStartExecutor extends SfdxCommandletExecutor<{}> {
  const errors = parser.getErrors();
  const pushedSource = successes ? successes.result.files : undefined;
  if (pushedSource || parser.hasConflicts()) {
- const rows = pushedSource || (errors && errors.data);
+ const rows = pushedSource || (errors && errors.files);
  const title = !parser.hasConflicts()
  ? nls.localize(`table_title_${titleType}ed_source`)
  : undefined;
@@ -207,9 +207,10 @@ export class ProjectDeployStartExecutor extends SfdxCommandletExecutor<{}> {
  }
 
  if (errors && !parser.hasConflicts()) {
- const { name, message, data } = errors;
- if (data) {
- const outputTable = this.getErrorTable(table, data, titleType);
+ const { name, message } = errors;
+ const files = errors.files;
+ if (files) {
+ const outputTable = this.getErrorTable(table, files, titleType);
  channelService.appendLine(outputTable);
  } else if (name && message) {
  channelService.appendLine(`${name}: ${message}\n`);

packages/salesforcedx-vscode-core/src/commands/util/sfdxCommandletExecutor.ts

@@ -18,7 +18,7 @@ import * as vscode from 'vscode';
 import { channelService } from '../../channels';
 import {
  FORCE_SOURCE_PULL_LOG_NAME,
- FORCE_SOURCE_PUSH_LOG_NAME
+ PROJECT_DEPLOY_START_LOG_NAME
 } from '../../constants';
 import { nls } from '../../messages';
 import { notificationService, ProgressNotification } from '../../notifications';
@@ -51,7 +51,7 @@ export abstract class SfdxCommandletExecutor<T>
  if (
  !(
  commandLogName === FORCE_SOURCE_PULL_LOG_NAME ||
- commandLogName === FORCE_SOURCE_PUSH_LOG_NAME
+ commandLogName === PROJECT_DEPLOY_START_LOG_NAME
  )
  ) {
  channelService.streamCommandOutput(execution);

packages/salesforcedx-vscode-core/src/constants.ts

@@ -64,5 +64,5 @@ export const FUNCTIONS_PATH = '/functions/';
 export const ORG_OPEN_COMMAND = 'sfdx.org.open';
 export const FORCE_SOURCE_PULL_LOG_NAME =
  'force_source_pull_default_scratch_org';
-export const FORCE_SOURCE_PUSH_LOG_NAME =
+export const PROJECT_DEPLOY_START_LOG_NAME =
  'project_deploy_start_default_scratch_org';

packages/salesforcedx-vscode-core/src/diagnostics/diagnostics.ts

@@ -16,34 +16,34 @@ import { SfdxCommandletExecutor } from '../commands/util';
 
 const notApplicable = 'N/A';
 
-export function getFileUri(
+export const getFileUri = (
  workspacePath: string,
  filePath: string,
  defaultErrorPath: string
-): string {
+): string => {
  const resolvedFilePath = filePath.includes(workspacePath)
  ? filePath
  : path.join(workspacePath, filePath);
  // source:deploy sometimes returns N/A as filePath
  return filePath === notApplicable ? defaultErrorPath : resolvedFilePath;
-}
+};
 
-export function getRange(
+export const getRange = (
  lineNumber: string,
  columnNumber: string
-): vscode.Range {
+): vscode.Range => {
  const ln = Number(lineNumber);
  const col = Number(columnNumber);
  const pos = new vscode.Position(ln > 0 ? ln - 1 : 0, col > 0 ? col - 1 : 0);
  return new vscode.Range(pos, pos);
-}
+};
 
-export function handleDiagnosticErrors(
+export const handlePushDiagnosticErrors = (
  errors: ProjectDeployStartErrorResponse,
  workspacePath: string,
  sourcePathOrPaths: string,
  errorCollection: vscode.DiagnosticCollection
-): vscode.DiagnosticCollection {
+): vscode.DiagnosticCollection => {
  errorCollection.clear();
 
  // In the case that we have deployed multiple source paths,
@@ -54,8 +54,8 @@ export function handleDiagnosticErrors(
  : sourcePathOrPaths;
 
  const diagnosticMap: Map<string, vscode.Diagnostic[]> = new Map();
- if (Reflect.has(errors, 'data')) {
- errors.data.forEach(error => {
+ if (Reflect.has(errors, 'files')) {
+ errors.files.forEach(error => {
  const fileUri = getFileUri(
  workspacePath,
  error.filePath,
@@ -84,7 +84,7 @@ export function handleDiagnosticErrors(
  const fileUri = vscode.Uri.file(file);
  errorCollection.set(fileUri, diagMap);
  });
- } else if (Reflect.has(errors, 'message')) {
+ } else if (Reflect.has(errors, 'status')) {
  const fileUri = vscode.Uri.file(defaultErrorPath);
  const range = getRange('1', '1');
  const diagnostic = {
@@ -98,12 +98,12 @@ export function handleDiagnosticErrors(
  }
 
  return errorCollection;
-}
+};
 
-export function handleDeployDiagnostics(
+export const handleDeployDiagnostics = (
  deployResult: DeployResult,
  errorCollection: vscode.DiagnosticCollection
-): vscode.DiagnosticCollection {
+): vscode.DiagnosticCollection => {
  errorCollection.clear();
  SfdxCommandletExecutor.errorCollection.clear();
 
@@ -144,18 +144,18 @@ export function handleDeployDiagnostics(
  );
 
  return errorCollection;
-}
+};
 
 // TODO: move to some type of file service or utility
-export function getAbsoluteFilePath(
+export const getAbsoluteFilePath = (
  filePath: string | undefined,
  workspacePath: string = getRootWorkspacePath()
-): string {
+): string => {
  let absoluteFilePath = filePath ?? workspacePath;
  if (!absoluteFilePath.includes(workspacePath)) {
  // Build the absolute filePath so that errors in the Problems
  // tab correctly link to the problem location in the file
  absoluteFilePath = [workspacePath, filePath].join('/');
  }
  return absoluteFilePath;
-}
+};

packages/salesforcedx-vscode-core/src/diagnostics/index.ts

@@ -9,6 +9,6 @@ export {
  getAbsoluteFilePath,
  getFileUri,
  getRange,
- handleDiagnosticErrors,
+ handlePushDiagnosticErrors,
  handleDeployDiagnostics
 } from './diagnostics';

packages/salesforcedx-vscode-core/test/vscode-integration/commands/projectDeployStartResultParser.test.ts

@@ -54,9 +54,8 @@ describe('Correctly output deploy results', () => {
  status: 1,
  name: 'Deploy Failed',
  message: 'There was a failure',
- stack: 'A stack',
  warnings: ['A warning'],
- data: [
+ files: [
  {
  filePath: 'src/classes/MyClass2.cls',
  error: 'Some Error'
@@ -133,7 +132,7 @@ describe('Correctly output deploy results', () => {
  'hasConflicts'
  ).returns(true);
  deployError.name = CONFLICT_ERROR_NAME;
- deployError.data = [
+ deployError.files = [
  {
  state: 'Conflict',
  fullName: 'SomeClass',
@@ -143,7 +142,7 @@ describe('Correctly output deploy results', () => {
  ];
  errorsStub.returns(deployError);
  const conflictsTable = table.createTable(
- deployError.data as unknown as Row[],
+ deployError.files as unknown as Row[],
  [
  { key: 'state', label: nls.localize('table_header_state') },
  { key: 'fullName', label: nls.localize('table_header_full_name') },

packages/salesforcedx-vscode-core/test/vscode-integration/diagnostics/index.test.ts

@@ -13,7 +13,7 @@ import {
  getAbsoluteFilePath,
  getFileUri,
  getRange,
- handleDiagnosticErrors
+ handlePushDiagnosticErrors
 } from '../../../src/diagnostics';
 
 describe('Diagnostics', () => {
@@ -27,10 +27,9 @@ describe('Diagnostics', () => {
  pushErrorResult = {
  message: 'Push failed.',
  name: 'PushFailed',
- stack: '123',
  status: 1,
  warnings: [],
- data: []
+ files: []
  };
  });
 
@@ -64,9 +63,9 @@ describe('Diagnostics', () => {
  fullName: 'Testing'
  };
 
- pushErrorResult.data.push(resultItem);
+ pushErrorResult.files.push(resultItem);
 
- handleDiagnosticErrors(
+ handlePushDiagnosticErrors(
  pushErrorResult,
  workspacePath,
  sourcePath,
@@ -107,10 +106,10 @@ describe('Diagnostics', () => {
  fullName: 'SomeController'
  };
 
- pushErrorResult.data.push(resultItem1);
- pushErrorResult.data.push(resultItem2);
+ pushErrorResult.files.push(resultItem1);
+ pushErrorResult.files.push(resultItem2);
 
- handleDiagnosticErrors(
+ handlePushDiagnosticErrors(
  pushErrorResult,
  workspacePath,
  sourcePath,
@@ -159,8 +158,8 @@ describe('Diagnostics', () => {
  type: 'ApexClass'
  };
 
- pushErrorResult.data.push(resultItem);
- handleDiagnosticErrors(
+ pushErrorResult.files.push(resultItem);
+ handlePushDiagnosticErrors(
  pushErrorResult,
  workspacePath,
  sourcePath,
@@ -194,17 +193,16 @@ describe('Diagnostics', () => {
  filePath: 'N/A'
  };
 
- pushErrorResult.data.push(resultItem1);
- pushErrorResult.data.push(resultItem2);
- handleDiagnosticErrors(
+ pushErrorResult.files.push(resultItem1);
+ pushErrorResult.files.push(resultItem2);
+ handlePushDiagnosticErrors(
  pushErrorResult,
  workspacePath,
  sourcePath,
  errorCollection
  );
 
  const testDiagnostics = languages.getDiagnostics(Uri.file(sourcePath));
-
  expect(testDiagnostics).to.be.an('array').to.have.lengthOf(2);
  expect(testDiagnostics[0].message).to.be.equals(resultItem1.error);
  expect(testDiagnostics[0].severity).to.be.equals(0); // vscode.DiagnosticSeverity.Error === 0