diff --git a/packages/code-analyzer-apexguru-engine/package.json b/packages/code-analyzer-apexguru-engine/package.json index 551d82a3..10ac6d9d 100644 --- a/packages/code-analyzer-apexguru-engine/package.json +++ b/packages/code-analyzer-apexguru-engine/package.json @@ -1,7 +1,7 @@ { "name": "@salesforce/code-analyzer-apexguru-engine", "description": "ApexGuru Engine Package for the Salesforce Code Analyzer", - "version": "0.39.0", + "version": "0.40.0-SNAPSHOT", "author": "The Salesforce Code Analyzer Team", "license": "BSD-3-Clause", "homepage": "https://developer.salesforce.com/docs/platform/salesforce-code-analyzer/overview", diff --git a/packages/code-analyzer-apexguru-engine/src/engine.ts b/packages/code-analyzer-apexguru-engine/src/engine.ts index f7c333f8..68b626dd 100644 --- a/packages/code-analyzer-apexguru-engine/src/engine.ts +++ b/packages/code-analyzer-apexguru-engine/src/engine.ts @@ -53,14 +53,6 @@ export class ApexGuruEngine extends EngineEventEmitter implements Engine { async describeRules(describeOptions: DescribeOptions): Promise { this.emitDescribeRulesProgressEvent(0); - // The SFAP API endpoint is environment-specific and supplied externally. - // When unset, this engine has nowhere to scan against, so it advertises no rules. - if (!process.env.SFAP_API_BASE_URL) { - this.emitLogEvent(LogLevel.Debug, 'SFAP API base URL not configured. ApexGuru engine is disabled.'); - this.emitDescribeRulesProgressEvent(100); - return []; - } - // Check if targeted files contain any Apex files if (describeOptions.workspace) { const targetedFiles = await describeOptions.workspace.getTargetedFiles(); @@ -97,15 +89,15 @@ export class ApexGuruEngine extends EngineEventEmitter implements Engine { // Get target org alias/username from config (passed by CLI --target-org flag) const targetOrg = this.getTargetOrg(); - // Initialize authentication + // Initialize authentication — skip gracefully if user is not authenticated try { await this.apexGuruService.initialize(targetOrg); } catch (error) { - const message = error instanceof Error ? error.message : String(error); - throw new Error( - `Failed to authenticate: ${message}\n` + - 'Please authenticate with: sf org login web' - ); + const detail = error instanceof Error ? error.message : String(error); + this.apexGuruService.cleanup(); + return this.skipWithError('NO_ORG_CONNECTION', + `Failed to authenticate: ${detail}`, + "Please authenticate with 'sf org login web' or pass --target-org"); } // Get workspace root path @@ -147,16 +139,59 @@ export class ApexGuruEngine extends EngineEventEmitter implements Engine { // Filter violations to only include selected rules const filteredViolations = allViolations.filter(v => selectedRulesSet.has(v.ruleName)); - // Return insights as scan metadata (workspace-level) - const insights: Record | undefined = scanMetadata ? { scan: scanMetadata } : undefined; + // Return insights with status: "completed" and scan metadata + const insights: Record = { + status: 'completed', + ...(scanMetadata ? { scan: scanMetadata } : {}) + }; return { violations: filteredViolations, insights }; + } catch (error) { + // Catch API failures (5xx, timeout, connection refused) and unexpected errors + const detail = error instanceof Error ? error.message : String(error); + if (this.isApiUnavailableError(error)) { + return this.skipWithError('API_UNAVAILABLE', + `ApexGuru service is unavailable: ${detail}`, + 'The ApexGuru service is temporarily unavailable. Please try again later.'); + } + return this.skipWithError('UNEXPECTED_ERROR', + `An unexpected error occurred: ${detail}`, + 'An unexpected error occurred. Please try again or file a support ticket if the issue persists.'); } finally { // Always cleanup resources this.apexGuruService.cleanup(); } } + /** + * Returns a graceful skip result with structured error insights. + * Emits a warn-level log event and completes progress before returning. + * NOTE: Caller is responsible for cleanup() — do NOT call cleanup here since + * this may be invoked from within a try-finally that already handles cleanup. + */ + private skipWithError(code: string, message: string, remediation: string): EngineRunResults { + this.emitLogEvent(LogLevel.Warn, `ApexGuru skipped: ${message}`); + this.emitRunRulesProgressEvent(100); + return { + violations: [], + insights: { + status: 'skipped', + error: { code, message, remediation } + } + }; + } + + /** + * Determines whether an error is an API unavailability issue (network/timeout/5xx). + */ + private isApiUnavailableError(error: unknown): boolean { + if (!(error instanceof Error)) return false; + const msg = error.message.toLowerCase(); + const networkIndicators = ['econnrefused', 'etimedout', 'enotfound', 'socket hang up', + 'connection refused', 'timeout', 'network', '502', '503', '504', '500']; + return networkIndicators.some(indicator => msg.includes(indicator)); + } + /** * Check if file is an Apex file based on extension */ diff --git a/packages/code-analyzer-apexguru-engine/src/services/ApexGuruAuthService.ts b/packages/code-analyzer-apexguru-engine/src/services/ApexGuruAuthService.ts index c745e058..443256db 100644 --- a/packages/code-analyzer-apexguru-engine/src/services/ApexGuruAuthService.ts +++ b/packages/code-analyzer-apexguru-engine/src/services/ApexGuruAuthService.ts @@ -40,7 +40,7 @@ export class ApexGuruAuthService { return; } catch (err) { const errorMessage = err instanceof Error ? err.message : String(err); - this.emitLogEvent(LogLevel.Error, `Failed to authenticate with org '${config.targetOrg}': ${errorMessage}`); + this.emitLogEvent(LogLevel.Fine, `Failed to authenticate with org '${config.targetOrg}': ${errorMessage}`); throw new Error( `Failed to authenticate with org '${config.targetOrg}'. ` + 'Please verify the org alias/username and ensure you are authenticated:\n' + @@ -58,7 +58,7 @@ export class ApexGuruAuthService { this.emitLogEvent(LogLevel.Fine, 'Successfully authenticated to default org'); } catch (err) { const errorMessage = err instanceof Error ? err.message : String(err); - this.emitLogEvent(LogLevel.Error, `Failed to authenticate: No default org found: ${errorMessage}`); + this.emitLogEvent(LogLevel.Fine, `Failed to authenticate: No default org found: ${errorMessage}`); throw new Error( 'No default org found. Please either:\n' + ' 1. Set a default org: sf config set target-org \n' + diff --git a/packages/code-analyzer-apexguru-engine/src/services/ApexGuruService.ts b/packages/code-analyzer-apexguru-engine/src/services/ApexGuruService.ts index bfb059a0..74c6bda8 100644 --- a/packages/code-analyzer-apexguru-engine/src/services/ApexGuruService.ts +++ b/packages/code-analyzer-apexguru-engine/src/services/ApexGuruService.ts @@ -27,7 +27,7 @@ export class ApexGuruService { private readonly initialRetryMs: number; private readonly maxRetryMs: number; private readonly backoffMultiplier: number; - private readonly sfapBaseUrl = process.env.SFAP_API_BASE_URL ?? ''; + private readonly sfapBaseUrl = 'https://dev.api.salesforce.com/platform/scale/v1-beta.1'; private progressCallback?: (progress: number) => void; private isCancelled = false; diff --git a/packages/code-analyzer-apexguru-engine/test/ApexGuruEngine.test.ts b/packages/code-analyzer-apexguru-engine/test/ApexGuruEngine.test.ts index 3e1224fd..0b98943d 100644 --- a/packages/code-analyzer-apexguru-engine/test/ApexGuruEngine.test.ts +++ b/packages/code-analyzer-apexguru-engine/test/ApexGuruEngine.test.ts @@ -1,6 +1,6 @@ import { ApexGuruEngine } from '../src/engine'; import { ApexGuruService } from '../src/services/ApexGuruService'; -import { RunOptions, Workspace } from '@salesforce/code-analyzer-engine-api'; +import { LogLevel, RunOptions, Workspace } from '@salesforce/code-analyzer-engine-api'; import * as fs from 'node:fs/promises'; // Mock dependencies @@ -11,12 +11,8 @@ describe('ApexGuruEngine', () => { let engine: ApexGuruEngine; let mockApexGuruService: jest.Mocked; let mockWorkspace: jest.Mocked; - let originalSfapBaseUrl: string | undefined; - beforeEach(() => { jest.clearAllMocks(); - originalSfapBaseUrl = process.env.SFAP_API_BASE_URL; - process.env.SFAP_API_BASE_URL = 'https://example.test/sfap'; mockApexGuruService = { initialize: jest.fn(), @@ -39,11 +35,7 @@ describe('ApexGuruEngine', () => { }); afterEach(() => { - if (originalSfapBaseUrl === undefined) { - delete process.env.SFAP_API_BASE_URL; - } else { - process.env.SFAP_API_BASE_URL = originalSfapBaseUrl; - } + jest.restoreAllMocks(); }); describe('getName', () => { @@ -63,17 +55,6 @@ describe('ApexGuruEngine', () => { }); describe('describeRules', () => { - it('should return empty array when SFAP_API_BASE_URL is not set', async () => { - delete process.env.SFAP_API_BASE_URL; - - const rules = await engine.describeRules({ - logFolder: '/tmp/logs', - workingFolder: '/tmp/working' - }); - - expect(rules).toEqual([]); - }); - it('should return all ApexGuru rules', async () => { const rules = await engine.describeRules({ logFolder: '/tmp/logs', @@ -156,6 +137,18 @@ describe('ApexGuruEngine', () => { expect(rules.length).toBeGreaterThan(0); expect(rules.find(r => r.name === 'SoqlInALoop')).toBeDefined(); }); + + it('should return rules without attempting authentication regardless of auth state', async () => { + const rules = await engine.describeRules({ + logFolder: '/tmp/logs', + workingFolder: '/tmp/working' + }); + + expect(rules.length).toBeGreaterThan(0); + expect(rules.find(r => r.name === 'SoqlInALoop')).toBeDefined(); + expect(rules.find(r => r.name === 'DmlInALoop')).toBeDefined(); + expect(mockApexGuruService.initialize).not.toHaveBeenCalled(); + }); }); describe('runRules', () => { @@ -189,12 +182,93 @@ describe('ApexGuruEngine', () => { expect(mockApexGuruService.scanWorkspace).toHaveBeenCalledWith('/test/workspace', ['/test/workspace']); }); - it('should throw error if authentication fails', async () => { - mockApexGuruService.initialize.mockRejectedValue(new Error('Auth failed')); + it('should gracefully skip with NO_ORG_CONNECTION when authentication fails', async () => { + mockApexGuruService.initialize.mockRejectedValue(new Error('No default org found')); mockWorkspace.getTargetedFiles.mockResolvedValue(['/test/workspace/Test.cls']); + const logSpy = jest.spyOn(engine as any, 'emitLogEvent'); + const progressSpy = jest.spyOn(engine as any, 'emitRunRulesProgressEvent'); - await expect(engine.runRules(['SoqlInALoop'], mockRunOptions)) - .rejects.toThrow('Failed to authenticate'); + const results = await engine.runRules(['SoqlInALoop'], mockRunOptions); + + expect(results.violations).toEqual([]); + expect(results.insights).toEqual({ + status: 'skipped', + error: { + code: 'NO_ORG_CONNECTION', + message: expect.stringContaining('No default org found'), + remediation: expect.stringContaining('sf org login web') + } + }); + expect(logSpy).toHaveBeenCalledWith( + LogLevel.Warn, + expect.stringContaining('Failed to authenticate') + ); + expect(mockApexGuruService.cleanup).toHaveBeenCalled(); + expect(progressSpy).toHaveBeenCalledWith(100); + // No SFAP calls should be made after auth failure + expect(mockApexGuruService.scanWorkspace).not.toHaveBeenCalled(); + }); + + it('should gracefully skip with API_UNAVAILABLE when API is unreachable', async () => { + mockWorkspace.getTargetedFiles.mockResolvedValue(['/test/workspace/Test.cls']); + mockApexGuruService.scanWorkspace.mockRejectedValue(new Error('connect ECONNREFUSED 127.0.0.1:443')); + const logSpy = jest.spyOn(engine as any, 'emitLogEvent'); + + const results = await engine.runRules(['SoqlInALoop'], mockRunOptions); + + expect(results.violations).toEqual([]); + expect(results.insights).toEqual({ + status: 'skipped', + error: { + code: 'API_UNAVAILABLE', + message: expect.stringContaining('ECONNREFUSED'), + remediation: expect.stringContaining('temporarily unavailable') + } + }); + expect(logSpy).toHaveBeenCalledWith( + LogLevel.Warn, + expect.stringContaining('unavailable') + ); + expect(mockApexGuruService.cleanup).toHaveBeenCalled(); + }); + + it('should gracefully skip with API_UNAVAILABLE on timeout', async () => { + mockWorkspace.getTargetedFiles.mockResolvedValue(['/test/workspace/Test.cls']); + mockApexGuruService.scanWorkspace.mockRejectedValue(new Error('Request timeout after 300000ms')); + const logSpy = jest.spyOn(engine as any, 'emitLogEvent'); + + const results = await engine.runRules(['SoqlInALoop'], mockRunOptions); + + expect(results.violations).toEqual([]); + expect(results.insights).toEqual({ + status: 'skipped', + error: { + code: 'API_UNAVAILABLE', + message: expect.stringContaining('timeout'), + remediation: expect.stringContaining('try again later') + } + }); + expect(logSpy).toHaveBeenCalledWith(LogLevel.Warn, expect.any(String)); + }); + + it('should gracefully skip with UNEXPECTED_ERROR on non-network errors', async () => { + mockWorkspace.getTargetedFiles.mockResolvedValue(['/test/workspace/Test.cls']); + mockApexGuruService.scanWorkspace.mockRejectedValue(new Error('Unexpected internal failure')); + const logSpy = jest.spyOn(engine as any, 'emitLogEvent'); + + const results = await engine.runRules(['SoqlInALoop'], mockRunOptions); + + expect(results.violations).toEqual([]); + expect(results.insights).toEqual({ + status: 'skipped', + error: { + code: 'UNEXPECTED_ERROR', + message: expect.stringContaining('Unexpected internal failure'), + remediation: expect.stringContaining('file a support ticket') + } + }); + expect(logSpy).toHaveBeenCalledWith(LogLevel.Warn, expect.any(String)); + expect(mockApexGuruService.cleanup).toHaveBeenCalled(); }); it('should return empty results if no Apex files found', async () => { @@ -346,12 +420,17 @@ describe('ApexGuruEngine', () => { it('should cleanup even when error occurs within try block', async () => { mockWorkspace.getTargetedFiles.mockResolvedValue(['/test/workspace/Test.cls']); - // Make scanWorkspace throw an error + // Make scanWorkspace throw an error — should be caught and return skip result mockApexGuruService.scanWorkspace.mockRejectedValue(new Error('Fatal API error')); - await expect(engine.runRules(['SoqlInALoop'], mockRunOptions)) - .rejects.toThrow('Fatal API error'); + const results = await engine.runRules(['SoqlInALoop'], mockRunOptions); + // Should not throw — error is caught and returned as UNEXPECTED_ERROR skip + expect(results.violations).toEqual([]); + expect(results.insights).toEqual({ + status: 'skipped', + error: expect.objectContaining({ code: 'UNEXPECTED_ERROR' }) + }); expect(mockApexGuruService.cleanup).toHaveBeenCalled(); }); @@ -383,7 +462,7 @@ describe('ApexGuruEngine', () => { expect(mockApexGuruService.initialize).toHaveBeenCalledWith('my-org'); }); - it('should populate insights in results when scanMetadata is returned', async () => { + it('should return insights with status completed and scan metadata on success', async () => { mockWorkspace.getTargetedFiles.mockResolvedValue(['/test/workspace/Test.cls']); const mockScanMetadata = { analysis_mode: 'full' as const, @@ -400,16 +479,19 @@ describe('ApexGuruEngine', () => { const results = await engine.runRules(['SoqlInALoop'], mockRunOptions); expect(results.insights).toBeDefined(); + expect(results.insights!['status']).toBe('completed'); expect(results.insights!['scan']).toEqual(mockScanMetadata); }); - it('should not include insights in results when no scanMetadata is returned', async () => { + it('should return insights with status completed even without scan metadata', async () => { mockWorkspace.getTargetedFiles.mockResolvedValue(['/test/workspace/Test.cls']); mockApexGuruService.scanWorkspace.mockResolvedValue({ violations: [] }); const results = await engine.runRules(['SoqlInALoop'], mockRunOptions); - expect(results.insights).toBeUndefined(); + expect(results.insights).toBeDefined(); + expect(results.insights!['status']).toBe('completed'); + expect(results.insights!['scan']).toBeUndefined(); }); it('should use file path from SFAP violation location', async () => { diff --git a/packages/code-analyzer-apexguru-engine/test/ApexGuruService.test.ts b/packages/code-analyzer-apexguru-engine/test/ApexGuruService.test.ts index adf323a4..d8a46b11 100644 --- a/packages/code-analyzer-apexguru-engine/test/ApexGuruService.test.ts +++ b/packages/code-analyzer-apexguru-engine/test/ApexGuruService.test.ts @@ -7,7 +7,7 @@ jest.mock('../src/services/ApexGuruAuthService'); jest.mock('archiver'); jest.mock('node:fs'); -const TEST_SFAP_BASE_URL = 'https://example.test/sfap'; +const TEST_SFAP_BASE_URL = 'https://dev.api.salesforce.com/platform/scale/v1-beta.1'; const mockFetch = jest.fn(); globalThis.fetch = mockFetch as unknown as typeof globalThis.fetch; @@ -16,12 +16,9 @@ describe('ApexGuruService', () => { let apexGuruService: ApexGuruService; let mockEmitLogEvent: jest.Mock; let mockAuthService: jest.Mocked; - let originalSfapBaseUrl: string | undefined; beforeEach(() => { jest.clearAllMocks(); - originalSfapBaseUrl = process.env.SFAP_API_BASE_URL; - process.env.SFAP_API_BASE_URL = TEST_SFAP_BASE_URL; mockEmitLogEvent = jest.fn(); mockAuthService = { @@ -45,11 +42,7 @@ describe('ApexGuruService', () => { }); afterEach(() => { - if (originalSfapBaseUrl === undefined) { - delete process.env.SFAP_API_BASE_URL; - } else { - process.env.SFAP_API_BASE_URL = originalSfapBaseUrl; - } + jest.restoreAllMocks(); }); describe('initialize', () => {