-
Notifications
You must be signed in to change notification settings - Fork 406
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: better exception handling for type inference errors #3127
Conversation
* @param packageDirs Package directory paths | ||
* @returns Relative path for the project | ||
*/ | ||
export function getRelativeProjectPath( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Moved to helper utils
import * as vscode from 'vscode'; | ||
import { sfdxCoreSettings } from '../../settings'; | ||
|
||
export function useBetaDeployRetrieve( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This function isn't really needed anymore - it was from when we needed to detect the type before deploying or retrieving in addition to checking the setting. Now we can just check the setting. I got rid of it now because it was actually causing problems - If we couldn't infer the type here no deploy would trigger at all.
const formattedException = new Error('Unknown Exception'); | ||
formattedException.name = e.name; | ||
|
||
for (const { fsPath } of uris) { | ||
const componentsForPath = ws.resolveSourceComponents(fsPath); | ||
if (supportedTypes && componentsForPath) { | ||
for (const component of componentsForPath) { | ||
if (!permittedTypeNames.has(component.type.name)) { | ||
return false; | ||
} | ||
} | ||
} | ||
if (e.name === 'TypeInferenceError') { | ||
const projectPath = getRelativeProjectPath( | ||
e.message.slice(0, e.message.lastIndexOf(':')), | ||
await SfdxPackageDirectories.getPackageDirectoryPaths() | ||
); | ||
formattedException.message = `${projectPath}: Could not infer metadata type`; | ||
} | ||
|
||
return true; | ||
return formattedException; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Convert to relative project path if it's a TypeInferenceError. Otherwise only throw the exception name with "Unknown Exception" as message.
createComponentCount, | ||
useBetaDeployRetrieve | ||
} from './betaDeployRetrieve'; | ||
export { createComponentCount } from './betaDeployRetrieve'; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We should also export formatException
here so we can import it elsewhere by just using the ./utils
path
Codecov Report
@@ Coverage Diff @@
## release/v51.8.0 #3127 +/- ##
===================================================
- Coverage 76.19% 76.18% -0.01%
===================================================
Files 276 276
Lines 10527 10511 -16
Branches 1243 1236 -7
===================================================
- Hits 8021 8008 -13
+ Misses 2157 2156 -1
+ Partials 349 347 -2
Continue to review full report at Codecov.
|
} | ||
} | ||
} | ||
if (e.name === 'TypeInferenceError') { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is there any chance that the error message does not follow this format (e.g. : ) ?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Only if we change the structure of the message :)
@@ -40,15 +41,15 @@ import { nls } from '../messages'; | |||
import { DeployQueue } from '../settings'; | |||
import { SfdxPackageDirectories, SfdxProjectConfig } from '../sfdxProject'; | |||
import { createComponentCount } from './util'; | |||
import { formatException } from './util/betaDeployRetrieve'; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We should be able to import this by just using ./util
path
What does this PR do?
Properly handles file paths when throwing TypeInfereceError exceptions. Also fixes it for when we are creating component counts in the baseDeployExecutor.
What issues does this PR fix or reference?
@W-9105085@