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

fix: display error message from response #608

Merged
merged 2 commits into from
Oct 17, 2022
Merged

Conversation

shetzel
Copy link
Contributor

@shetzel shetzel commented Oct 13, 2022

What does this PR do?

When a deploy fails without any component failures the command was not showing the errorMessage field that is set on the response. This PR adds that error message to the error thrown by the command.

What issues does this PR fix or reference?

@W-11905229@

For Testing:
Deploy code that will cause a GACK. I have a repro for you.

sfdx force:source:deploy -p force-app
sfdx force:mdapi:deploy
sfdx force:source:push

Copy link
Contributor

@mshanemc mshanemc left a comment

Choose a reason for hiding this comment

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

approved. 1 question and a non-blocking suggestion

Comment on lines +89 to +94
let errMsg = '';
this.results?.forEach((res) => {
if (res.response?.errorMessage) {
errMsg += `${res.response?.errorMessage}\n`;
}
});
Copy link
Contributor

Choose a reason for hiding this comment

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

Is \n fine across os ? Or do you need the EOL thing?

Suggested change
let errMsg = '';
this.results?.forEach((res) => {
if (res.response?.errorMessage) {
errMsg += `${res.response?.errorMessage}\n`;
}
});
const errMsg = this.results?.filter((res) => res.response?.errorMessage).join('\n')

@@ -102,6 +102,26 @@ describe('PushResultFormatter', () => {
expect(headerStub.callCount, JSON.stringify(headerStub.args)).to.equal(1);
expect(tableStub.callCount, JSON.stringify(tableStub.args)).to.equal(1);
});
it('should output as expected for a deploy failure (GACK)', async () => {
const errorMessage =
'UNKNOWN_EXCEPTION: An unexpected error occurred. Please include this ErrorId if you contact support: 1730955361-49792 (-1117026034)';
Copy link
Contributor

Choose a reason for hiding this comment

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

nice

@mshanemc
Copy link
Contributor

mshanemc commented Oct 17, 2022

QA notes

repro'd per Steves fork and instructions. This was actually a failure:

➜  functions-recipes-bug git:(main) ✗ sfdx force:source:push -f      
*** Pushing with SOAP API v55.0 ***
Updating source tracking... done

this branch...never been so excited to see a GACK ✅

➜  functions-recipes-bug git:(main) ✗ ../../plugin-source/bin/dev force:source:push -f
*** Pushing with SOAP API v55.0 ***
ERROR running force:source:push:  Push failed. UNKNOWN_EXCEPTION: An unexpected error occurred. Please include this ErrorId if you contact support: 1060938082-94495 (1106287627)

✅ also see a gack via force:source:deploy -p

@mshanemc mshanemc merged commit 6ba254e into main Oct 17, 2022
@mshanemc mshanemc deleted the sh/additional-error-messages branch October 17, 2022 21:29
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.

2 participants