Skip to content

Commit eef35a1

Browse files
data-douserdata-douser
committed
Improve CDS extractor non-fatal error handling
Improves the handling of non-fatal errors encountered during a given CDS extractor run by extending the 'codeql database add-diagnostic' pattern for existing code where errors or warnings are caught and logged. Refactors some unit tests and adds more unit tests to cover the code changes for non-fatal error handling via database diagnostics. Updates CDS extractor build (ts and jest) configs in order to avoid console logging in unit test output and, overall, produce cleaner test output.
1 parent cbb69d0 commit eef35a1

File tree

12 files changed

+557
-183
lines changed

12 files changed

+557
-183
lines changed

‎extractors/cds/tools/index-files.ts‎

Lines changed: 5 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,6 @@
1-
import { addCompilationDiagnostic, compileCdsToJson, determineCdsCommand } from './src/cdsCompiler';
1+
import { compileCdsToJson, determineCdsCommand } from './src/cdsCompiler';
22
import { runJavaScriptExtractor } from './src/codeql';
3+
import { addCompilationDiagnostic } from './src/diagnostics';
34
import { configureLgtmIndexFilters, setupAndValidateEnvironment } from './src/environment';
45
import { getCdsFilePathsToProcess } from './src/filesystem';
56
import { findPackageJsonDirs, installDependencies } from './src/packageManager';
@@ -53,11 +54,11 @@ if (!filePathsResult.success) {
5354
const cdsFilePathsToProcess = filePathsResult.cdsFilePaths;
5455

5556
// Find all package.json directories that have a `@sap/cds` node dependency.
56-
const packageJsonDirs = findPackageJsonDirs(cdsFilePathsToProcess);
57+
const packageJsonDirs = findPackageJsonDirs(cdsFilePathsToProcess, codeqlExePath);
5758

5859
// Install node dependencies in each directory.
5960
console.log('Pre-installing required CDS compiler versions ...');
60-
installDependencies(packageJsonDirs);
61+
installDependencies(packageJsonDirs, codeqlExePath);
6162

6263
// Determine the CDS command to use.
6364
const cdsCommand = determineCdsCommand();
@@ -88,7 +89,7 @@ for (const rawCdsFilePath of cdsFilePathsToProcess) {
8889
configureLgtmIndexFilters();
8990

9091
// Run CodeQL's JavaScript extractor to process the compiled JSON files.
91-
const extractorResult = runJavaScriptExtractor(sourceRoot, autobuildScriptPath);
92+
const extractorResult = runJavaScriptExtractor(sourceRoot, autobuildScriptPath, codeqlExePath);
9293
if (!extractorResult.success && extractorResult.error) {
9394
console.error(`Error running JavaScript extractor: ${extractorResult.error}`);
9495
}

‎extractors/cds/tools/jest.config.js‎

Lines changed: 7 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -4,26 +4,22 @@ module.exports = {
44
testEnvironment: 'node',
55
testMatch: ['**/test/**/*.test.ts'],
66
roots: ['<rootDir>/src/', '<rootDir>/test/'],
7-
collectCoverageFrom: [
8-
'src/**/*.ts',
9-
'!src/**/*.d.ts',
10-
'!**/node_modules/**',
11-
],
7+
collectCoverageFrom: ['src/**/*.ts', '!src/**/*.d.ts', '!**/node_modules/**'],
128
coverageReporters: ['text', 'lcov', 'clover', 'json'],
139
coverageDirectory: 'coverage',
1410
verbose: true,
1511
moduleDirectories: ['node_modules', 'src'],
1612
moduleFileExtensions: ['ts', 'js', 'json', 'node'],
13+
setupFilesAfterEnv: ['<rootDir>/test/jest.setup.ts'],
1714
transform: {
1815
'^.+\\.ts$': [
1916
'ts-jest',
2017
{
2118
tsconfig: 'tsconfig.json',
2219
diagnostics: {
23-
warnOnly: true
20+
warnOnly: true,
2421
},
25-
isolatedModules: true,
26-
}
27-
]
28-
}
29-
};
22+
},
23+
],
24+
},
25+
};

‎extractors/cds/tools/src/cdsCompiler.ts‎

Lines changed: 27 additions & 69 deletions
Original file line numberDiff line numberDiff line change
@@ -1,8 +1,6 @@
11
import { execFileSync, spawnSync, SpawnSyncReturns } from 'child_process';
22
import { resolve } from 'path';
33

4-
import * as shellQuote from 'shell-quote';
5-
64
import { fileExists, dirExists, recursivelyRenameJsonFiles } from './filesystem';
75

86
/**
@@ -14,39 +12,6 @@ export interface CdsCompilationResult {
1412
outputPath?: string;
1513
}
1614

17-
/**
18-
* Determine the `cds` command to use based on the environment.
19-
* @returns A string representing the CLI command to run to invoke the
20-
* CDS compiler.
21-
*/
22-
export function determineCdsCommand(): string {
23-
let cdsCommand = 'cds';
24-
// TODO : create a mapping of project sub-directories to the correct
25-
// cds command to use, which will also determine the version of the cds
26-
// compiler that will be used for compiling `.cds` files to `.cds.json`
27-
// files for that sub-directory / project.
28-
try {
29-
execFileSync('cds', ['--version'], { stdio: 'ignore' });
30-
} catch (error) {
31-
// Check if the error is specifically about the command not being found
32-
const errorMsg = String(error);
33-
if (errorMsg.includes('command not found')) {
34-
// If 'cds' command is not available, use npx to run it
35-
console.log('CDS command not found, falling back to npx...');
36-
} else if (errorMsg.includes('ENOENT') || errorMsg.includes('not recognized')) {
37-
// If the error is related to the command not being recognized, use npx
38-
console.log('CDS command not recognized, falling back to npx...');
39-
} else {
40-
// For other errors, log them but still fall back to npx
41-
console.warn(
42-
`WARN: determining CDS command failed with error: ${errorMsg}. Falling back to npx...`,
43-
);
44-
}
45-
cdsCommand = 'npx -y --package @sap/cds-dk cds';
46-
}
47-
return cdsCommand;
48-
}
49-
5015
/**
5116
* Compile a CDS file to JSON
5217
* @param cdsFilePath Path to the CDS file
@@ -116,42 +81,35 @@ export function compileCdsToJson(
11681
return { success: false, message: String(error) };
11782
}
11883
}
119-
12084
/**
121-
* Add a diagnostic error to the CodeQL database for a failed CDS compilation
122-
* @param cdsFilePath Path to the CDS file that failed to compile
123-
* @param errorMessage The error message from the compilation
124-
* @param codeqlExePath Path to the CodeQL executable
125-
* @returns True if the diagnostic was added, false otherwise
85+
* Determine the `cds` command to use based on the environment.
86+
* @returns A string representing the CLI command to run to invoke the
87+
* CDS compiler.
12688
*/
127-
export function addCompilationDiagnostic(
128-
cdsFilePath: string,
129-
errorMessage: string,
130-
codeqlExePath: string,
131-
): boolean {
89+
export function determineCdsCommand(): string {
90+
let cdsCommand = 'cds';
91+
// TODO : create a mapping of project sub-directories to the correct
92+
// cds command to use, which will also determine the version of the cds
93+
// compiler that will be used for compiling `.cds` files to `.cds.json`
94+
// files for that sub-directory / project.
13295
try {
133-
// Use shell-quote to safely escape the error message
134-
const escapedErrorMessage = shellQuote.quote([errorMessage]);
135-
136-
execFileSync(codeqlExePath, [
137-
'database',
138-
'add-diagnostic',
139-
'--extractor-name=cds',
140-
'--ready-for-status-page',
141-
'--source-id=cds/compilation-failure',
142-
'--source-name=Failure to compile one or more SAP CAP CDS files',
143-
'--severity=error',
144-
`--markdown-message=${escapedErrorMessage.slice(1, -1)}`, // Remove the added quotes from shell-quote
145-
`--file-path=${resolve(cdsFilePath)}`,
146-
'--',
147-
`${process.env.CODEQL_EXTRACTOR_CDS_WIP_DATABASE ?? ''}`,
148-
]);
149-
console.log(`Added error diagnostic for source file: ${cdsFilePath}`);
150-
return true;
151-
} catch (err) {
152-
console.error(
153-
`ERROR: Failed to add error diagnostic for source file=${cdsFilePath} : ${String(err)}`,
154-
);
155-
return false;
96+
execFileSync('cds', ['--version'], { stdio: 'ignore' });
97+
} catch (error) {
98+
// Check if the error is specifically about the command not being found
99+
const errorMsg = String(error);
100+
if (errorMsg.includes('command not found')) {
101+
// If 'cds' command is not available, use npx to run it
102+
console.log('CDS command not found, falling back to npx...');
103+
} else if (errorMsg.includes('ENOENT') || errorMsg.includes('not recognized')) {
104+
// If the error is related to the command not being recognized, use npx
105+
console.log('CDS command not recognized, falling back to npx...');
106+
} else {
107+
// For other errors, log them but still fall back to npx
108+
console.warn(
109+
`WARN: determining CDS command failed with error: ${errorMsg}. Falling back to npx...`,
110+
);
111+
}
112+
cdsCommand = 'npx -y --package @sap/cds-dk cds';
156113
}
114+
return cdsCommand;
157115
}

‎extractors/cds/tools/src/codeql.ts‎

Lines changed: 13 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,17 +1,20 @@
11
import { spawnSync, SpawnSyncReturns } from 'child_process';
22
import { existsSync } from 'fs';
33

4+
import { addJavaScriptExtractorDiagnostic } from './diagnostics';
45
import { getPlatformInfo } from './environment';
56

67
/**
78
* Run the JavaScript extractor autobuild script
89
* @param sourceRoot The source root directory
910
* @param autobuildScriptPath Path to the autobuild script
11+
* @param codeqlExePath Path to the CodeQL executable (optional)
1012
* @returns Success status and any error message
1113
*/
1214
export function runJavaScriptExtractor(
1315
sourceRoot: string,
1416
autobuildScriptPath: string,
17+
codeqlExePath?: string,
1518
): { success: boolean; error?: string } {
1619
console.log(
1720
`Extracting the .cds.json files by running the 'javascript' extractor autobuild script:
@@ -38,16 +41,24 @@ export function runJavaScriptExtractor(
3841
});
3942

4043
if (result.error) {
44+
const errorMessage = `Error executing JavaScript extractor: ${result.error.message}`;
45+
if (codeqlExePath) {
46+
addJavaScriptExtractorDiagnostic(sourceRoot, errorMessage, codeqlExePath);
47+
}
4148
return {
4249
success: false,
43-
error: `Error executing JavaScript extractor: ${result.error.message}`,
50+
error: errorMessage,
4451
};
4552
}
4653

4754
if (result.status !== 0) {
55+
const errorMessage = `JavaScript extractor failed with exit code: ${String(result.status)}`;
56+
if (codeqlExePath) {
57+
addJavaScriptExtractorDiagnostic(sourceRoot, errorMessage, codeqlExePath);
58+
}
4859
return {
4960
success: false,
50-
error: `JavaScript extractor failed with exit code: ${String(result.status)}`,
61+
error: errorMessage,
5162
};
5263
}
5364

‎extractors/cds/tools/src/diagnostics.ts‎

Lines changed: 153 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,153 @@
1+
import { execFileSync } from 'child_process';
2+
import { resolve } from 'path';
3+
4+
import { quote } from 'shell-quote';
5+
6+
/**
7+
* Severity levels for diagnostics
8+
*/
9+
export enum DiagnosticSeverity {
10+
Error = 'error',
11+
Warning = 'warning',
12+
Note = 'note',
13+
Recommendation = 'recommendation',
14+
}
15+
16+
/**
17+
* Base function to add a diagnostic to the CodeQL database
18+
* @param filePath Path to the file related to the diagnostic
19+
* @param message The diagnostic message
20+
* @param codeqlExePath Path to the CodeQL executable
21+
* @param sourceId The source ID for the diagnostic
22+
* @param sourceName The source name for the diagnostic
23+
* @param severity The severity level of the diagnostic
24+
* @param logPrefix Prefix for the log message
25+
* @returns True if the diagnostic was added, false otherwise
26+
*/
27+
export function addDiagnostic(
28+
filePath: string,
29+
message: string,
30+
codeqlExePath: string,
31+
sourceId: string,
32+
sourceName: string,
33+
severity: DiagnosticSeverity,
34+
logPrefix: string,
35+
): boolean {
36+
try {
37+
// Use shell-quote to safely escape the message
38+
const escapedMessage = quote([message]);
39+
40+
execFileSync(codeqlExePath, [
41+
'database',
42+
'add-diagnostic',
43+
'--extractor-name=cds',
44+
'--ready-for-status-page',
45+
`--source-id=${sourceId}`,
46+
`--source-name=${sourceName}`,
47+
`--severity=${severity}`,
48+
`--markdown-message=${escapedMessage.slice(1, -1)}`, // Remove the added quotes from shell-quote
49+
`--file-path=${resolve(filePath)}`,
50+
'--',
51+
`${process.env.CODEQL_EXTRACTOR_CDS_WIP_DATABASE ?? ''}`,
52+
]);
53+
console.log(`Added ${severity} diagnostic for ${logPrefix}: ${filePath}`);
54+
return true;
55+
} catch (err) {
56+
console.error(
57+
`ERROR: Failed to add ${severity} diagnostic for ${logPrefix}=${filePath} : ${String(err)}`,
58+
);
59+
return false;
60+
}
61+
}
62+
63+
/**
64+
* Add a diagnostic error to the CodeQL database for a failed CDS compilation
65+
* @param cdsFilePath Path to the CDS file that failed to compile
66+
* @param errorMessage The error message from the compilation
67+
* @param codeqlExePath Path to the CodeQL executable
68+
* @returns True if the diagnostic was added, false otherwise
69+
*/
70+
export function addCompilationDiagnostic(
71+
cdsFilePath: string,
72+
errorMessage: string,
73+
codeqlExePath: string,
74+
): boolean {
75+
return addDiagnostic(
76+
cdsFilePath,
77+
errorMessage,
78+
codeqlExePath,
79+
'cds/compilation-failure',
80+
'Failure to compile one or more SAP CAP CDS files',
81+
DiagnosticSeverity.Error,
82+
'source file',
83+
);
84+
}
85+
86+
/**
87+
* Add a diagnostic error to the CodeQL database for a dependency installation failure
88+
* @param packageJsonPath Path to the package.json file that has installation issues
89+
* @param errorMessage The error message from the installation
90+
* @param codeqlExePath Path to the CodeQL executable
91+
* @returns True if the diagnostic was added, false otherwise
92+
*/
93+
export function addDependencyDiagnostic(
94+
packageJsonPath: string,
95+
errorMessage: string,
96+
codeqlExePath: string,
97+
): boolean {
98+
return addDiagnostic(
99+
packageJsonPath,
100+
errorMessage,
101+
codeqlExePath,
102+
'cds/dependency-failure',
103+
'Failure to install SAP CAP CDS dependencies',
104+
DiagnosticSeverity.Error,
105+
'package.json file',
106+
);
107+
}
108+
109+
/**
110+
* Add a diagnostic warning to the CodeQL database for a package.json parsing failure
111+
* @param packageJsonPath Path to the package.json file that couldn't be parsed
112+
* @param errorMessage The error message from the parsing attempt
113+
* @param codeqlExePath Path to the CodeQL executable
114+
* @returns True if the diagnostic was added, false otherwise
115+
*/
116+
export function addPackageJsonParsingDiagnostic(
117+
packageJsonPath: string,
118+
errorMessage: string,
119+
codeqlExePath: string,
120+
): boolean {
121+
return addDiagnostic(
122+
packageJsonPath,
123+
errorMessage,
124+
codeqlExePath,
125+
'cds/package-json-parsing-failure',
126+
'Failure to parse package.json file for SAP CAP CDS project',
127+
DiagnosticSeverity.Warning,
128+
'package.json file',
129+
);
130+
}
131+
132+
/**
133+
* Add a diagnostic error to the CodeQL database for a JavaScript extractor failure
134+
* @param filePath Path to a relevant file for the error context
135+
* @param errorMessage The error message from the JavaScript extractor
136+
* @param codeqlExePath Path to the CodeQL executable
137+
* @returns True if the diagnostic was added, false otherwise
138+
*/
139+
export function addJavaScriptExtractorDiagnostic(
140+
filePath: string,
141+
errorMessage: string,
142+
codeqlExePath: string,
143+
): boolean {
144+
return addDiagnostic(
145+
filePath,
146+
errorMessage,
147+
codeqlExePath,
148+
'cds/js-extractor-failure',
149+
'Failure in JavaScript extractor for SAP CAP CDS files',
150+
DiagnosticSeverity.Error,
151+
'extraction file',
152+
);
153+
}

0 commit comments

Comments
 (0)