Skip to content

Commit

Permalink
fix: try/finally on pollStatus to handle throws from timeout (#538)
Browse files Browse the repository at this point in the history
* fix: try/finally on pollStatus to handle throws from timeout

* fix: use try/catch

* fix: source:deploy:report needs the finally - good catch unit tests!

* Update src/commands/force/mdapi/deploy/report.ts

Co-authored-by: Shane McLaughlin <[email protected]>

* fix: review updates

Co-authored-by: Steve Hetzel <[email protected]>
  • Loading branch information
mshanemc and shetzel authored Jul 18, 2022
1 parent b4d8a5c commit 68ff738
Show file tree
Hide file tree
Showing 3 changed files with 37 additions and 9 deletions.
17 changes: 13 additions & 4 deletions src/commands/force/mdapi/deploy/report.ts
Original file line number Diff line number Diff line change
Expand Up @@ -82,20 +82,29 @@ export class Report extends DeployCommand {
);

if (this.isAsync) {
this.deployResult = await this.report(this.getFlag<string>('jobid'));
this.deployResult = await this.report(deployId);
return;
}

this.displayDeployId(deployId);

const deploy = this.createDeploy(deployId);
if (!this.isJsonOutput()) {
const progressFormatter: ProgressFormatter = env.getBoolean('SFDX_USE_PROGRESS_BAR', true)
? new DeployProgressBarFormatter(this.logger, this.ux)
: new DeployProgressStatusFormatter(this.logger, this.ux);
progressFormatter.progress(deploy);
}
this.deployResult = await deploy.pollStatus({ frequency: Duration.milliseconds(500), timeout: waitDuration });

try {
this.displayDeployId(deployId);
this.deployResult = await deploy.pollStatus({ frequency: Duration.milliseconds(500), timeout: waitDuration });
} catch (error) {
if (error instanceof Error && error.message.includes('The client has timed out')) {
this.logger.debug('mdapi:deploy:report polling timed out. Requesting status...');
this.deployResult = await this.report(deployId);
} else {
throw error;
}
}
}

// this is different from the source:report uses report error codes (unfortunately)
Expand Down
13 changes: 11 additions & 2 deletions src/commands/force/source/deploy/report.ts
Original file line number Diff line number Diff line change
Expand Up @@ -93,8 +93,17 @@ export class Report extends DeployCommand {
: new DeployProgressStatusFormatter(this.logger, this.ux);
progressFormatter.progress(deploy);
}
await deploy.pollStatus({ timeout: waitDuration });
this.deployResult = await this.report(deployId);
try {
await deploy.pollStatus({ timeout: waitDuration });
} catch (error) {
if (error instanceof Error && error.message.includes('The client has timed out')) {
this.logger.debug('source:deploy:report polling timed out. Requesting status...');
} else {
throw error;
}
} finally {
this.deployResult = await this.report(deployId);
}
}

// No-op implementation since any DeployResult status would be a success.
Expand Down
16 changes: 13 additions & 3 deletions test/commands/source/report.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@ import { DeployCommandResult } from '../../../src/formatters/deployResultFormatt
import { DeployProgressBarFormatter } from '../../../src/formatters/deployProgressBarFormatter';
import { DeployProgressStatusFormatter } from '../../../src/formatters/deployProgressStatusFormatter';
import { Stash } from '../../../src/stash';
import { getDeployResult } from './deployResponses';
import { getDeployResult, getDeployResponse } from './deployResponses';

describe('force:source:report', () => {
const sandbox = sinon.createSandbox();
Expand Down Expand Up @@ -56,7 +56,6 @@ describe('force:source:report', () => {
}

public createDeploy(): MetadataApiDeploy {
pollStatusStub = sandbox.stub(MetadataApiDeploy.prototype, 'pollStatus');
return MetadataApiDeploy.prototype;
}
}
Expand Down Expand Up @@ -86,13 +85,14 @@ describe('force:source:report', () => {
});
uxLogStub = stubMethod(sandbox, UX.prototype, 'log');
stubMethod(sandbox, ConfigFile.prototype, 'get').returns({ jobid: stashedDeployId });
checkDeployStatusStub = sandbox.stub().resolves(expectedResults);

return cmd.runIt();
};

beforeEach(() => {
readSyncStub = stubMethod(sandbox, ConfigFile.prototype, 'readSync');
pollStatusStub = sandbox.stub(MetadataApiDeploy.prototype, 'pollStatus');
checkDeployStatusStub = sandbox.stub().resolves(expectedResults);
});

afterEach(() => {
Expand Down Expand Up @@ -174,4 +174,14 @@ describe('force:source:report', () => {
expect(progressBarStub.calledOnce).to.equal(true);
expect(pollStatusStub.calledOnce).to.equal(true);
});

it('should NOT error with client timed out when --wait > 0', async () => {
const inProgressDeployResult = getDeployResponse('inProgress');
pollStatusStub.reset();
pollStatusStub.throws(Error('The client has timed out'));
checkDeployStatusStub.reset();
checkDeployStatusStub.resolves(inProgressDeployResult);
const result = await runReportCmd(['--json', '--wait', '1']);
expect(result).to.deep.equal(inProgressDeployResult);
});
});

0 comments on commit 68ff738

Please sign in to comment.