NEW @W-21910054@ Message sanitization - #500
Conversation
|
Git2Gus App is installed but the |
aruntyagiTutu
left a comment
There was a problem hiding this comment.
The message-sanitization changes themselves look right — dropping the leaked SFAP API returned ... prefix from formatHttpError, rewording the auth-failure message to be more actionable, retitling the INVALID_SESSION/SCAN_TIMEOUT copy to be consistent with the wording already shipped in sfdx-code-analyzer-vscode#368, and the new formatSeconds() helper for human-readable timeout messages are all reasonable and covered by updated tests.
However, packages/code-analyzer-apexguru-engine/src/config.ts also changes the default api_timeout_ms from 300000 to 3000 while leaving the // 5 minutes comment unchanged:
- api_timeout_ms: 300000, // 5 minutes
+ api_timeout_ms: 3000, // 5 minutesThis looks like an unintentional typo (dropped two zeros) rather than a deliberate change — the PR description is empty and gives no rationale, the title/scope is "Message sanitization," and nothing else in the diff references timeout tuning. As written, this drops the default ApexGuru scan timeout from 5 minutes to 3 seconds, which would cause real scans to time out almost immediately for anyone relying on the default. It isn't caught by ApexGuruEngine.test.ts because that test hardcodes 300000 explicitly rather than exercising DEFAULT_APEXGURU_ENGINE_CONFIG.
Could you confirm whether this was intentional? If not, please restore 300000. If it was intentional, it should be called out explicitly in the PR description and probably split out from the sanitization change.
aruntyagiTutu
left a comment
There was a problem hiding this comment.
Thanks for the fix — confirmed the timeout default is now 600000 (10 minutes), with the comment, config description, and test assertion ("600 seconds") all updated consistently. That resolves my earlier concern about the accidental 3-second default. LGTM.
No description provided.