diff --git a/packages/code-analyzer-flow-engine/FlowScanner/flow_parser/parse.py b/packages/code-analyzer-flow-engine/FlowScanner/flow_parser/parse.py index b14dd479..2eb617b5 100644 --- a/packages/code-analyzer-flow-engine/FlowScanner/flow_parser/parse.py +++ b/packages/code-analyzer-flow-engine/FlowScanner/flow_parser/parse.py @@ -763,7 +763,14 @@ def get_run_mode(self) -> RunMode: if len(elems) == 0: return RunMode.DefaultMode else: - return RunMode[elems[0].text] + declared = (elems[0].text or '').strip() + try: + return RunMode[declared] + except KeyError: + # A runInMode value we don't know about (for example, one added in a newer API version) must not crash + # the whole scan. Fall back to the default (user context) behavior and say so in the log. + logger.warning(f"Unrecognized runInMode '{declared}' in {self.flow_path}, treating it as DefaultMode") + return RunMode.DefaultMode def get_api_version(self) -> str: """Get the API version of the flow. @@ -1085,6 +1092,10 @@ def build_vartype_from_elem(elem: El) -> VariableType | None: elif tag == 'decisions': return VariableType(tag=tag, reference=ReferenceType.NodeReference) + elif tag == 'ends': + # API v68+ top-level End element. Terminal node, holds no value. + return VariableType(tag=tag, reference=ReferenceType.NodeReference) + elif tag == 'dynamicChoiceSets': # These are effectively record lookups # TODO: handle this better, right now we just have a stub diff --git a/packages/code-analyzer-flow-engine/FlowScanner/public/enums.py b/packages/code-analyzer-flow-engine/FlowScanner/public/enums.py index 1e0505f9..3e3812f2 100644 --- a/packages/code-analyzer-flow-engine/FlowScanner/public/enums.py +++ b/packages/code-analyzer-flow-engine/FlowScanner/public/enums.py @@ -89,6 +89,7 @@ class RunMode(Enum): SystemModeWithoutSharing = 0 SystemModeWithSharing = 1 DefaultMode = 2 + UserMode = 3 # API v68+: runInMode=UserMode. Runs in user context, so it behaves like DefaultMode. class DataType(Enum): diff --git a/packages/code-analyzer-flow-engine/FlowScanner/public/parse_utils.py b/packages/code-analyzer-flow-engine/FlowScanner/public/parse_utils.py index 7c06d28c..ea9b8cab 100644 --- a/packages/code-analyzer-flow-engine/FlowScanner/public/parse_utils.py +++ b/packages/code-analyzer-flow-engine/FlowScanner/public/parse_utils.py @@ -80,6 +80,7 @@ "collectionProcessors", "customErrors", "decisions", + "ends", "loops", "orchestratedStages", "recordLookups", diff --git a/packages/code-analyzer-flow-engine/FlowScanner/tests/test_parse_utils.py b/packages/code-analyzer-flow-engine/FlowScanner/tests/test_parse_utils.py index eb7fc58e..3fea3341 100644 --- a/packages/code-analyzer-flow-engine/FlowScanner/tests/test_parse_utils.py +++ b/packages/code-analyzer-flow-engine/FlowScanner/tests/test_parse_utils.py @@ -71,5 +71,18 @@ def test_missing_file_returns_none(self): self.assertIsNone(pu.quick_validate(self.flow_path + '.does-not-exist')) +class EndsElementTest(unittest.TestCase): + """Regression tests for issue #2098: API v68 top-level elements.""" + + def test_ends_is_control_flow_element(self): + self.assertIn('ends', pu.CTRL_FLOW_ELEM) + + def test_ends_has_variable_type(self): + import xml.etree.ElementTree as ET + from flow_parser import parse + elem = ET.fromstring('End1') + self.assertIsNotNone(parse.build_vartype_from_elem(elem)) + + if __name__ == '__main__': unittest.main() diff --git a/packages/code-analyzer-flow-engine/FlowScanner/tests/test_run_mode.py b/packages/code-analyzer-flow-engine/FlowScanner/tests/test_run_mode.py new file mode 100644 index 00000000..af3109ec --- /dev/null +++ b/packages/code-analyzer-flow-engine/FlowScanner/tests/test_run_mode.py @@ -0,0 +1,64 @@ +"""Tests for how the parser handles the flow's runInMode element. + +Run from the FlowScanner directory with:: + + python3 -m unittest tests.test_run_mode +""" +import os +import tempfile +import unittest + +from flow_parser.parse import Parser +from public.enums import RunMode +from flow_scanner import util + +FLOW_TEMPLATE = ( + '\n' + '' + '68.0' + '' + 'AutoLaunchedFlow' + '{run_in_mode}' + 'Active' + '00' + '' +) + + +def parse_flow_with_run_in_mode(run_in_mode_value): + """Writes a minimal flow to disk and returns its parser's declared run mode.""" + element = '' if run_in_mode_value is None else f'{run_in_mode_value}' + with tempfile.TemporaryDirectory() as tmp: + path = os.path.join(tmp, 'RunModeTest.flow-meta.xml') + with open(path, 'w', encoding='utf-8') as f: + f.write(FLOW_TEMPLATE.format(run_in_mode=element)) + return Parser.from_file(path).get_declared_run_mode() + + +class RunInModeTest(unittest.TestCase): + """Regression tests for API v68's runInMode=UserMode, which used to raise KeyError and crash the scan.""" + + def test_user_mode_is_recognized(self): + self.assertIs(parse_flow_with_run_in_mode('UserMode'), RunMode.UserMode) + + def test_existing_modes_are_unchanged(self): + self.assertIs(parse_flow_with_run_in_mode('SystemModeWithoutSharing'), RunMode.SystemModeWithoutSharing) + self.assertIs(parse_flow_with_run_in_mode('SystemModeWithSharing'), RunMode.SystemModeWithSharing) + self.assertIs(parse_flow_with_run_in_mode('DefaultMode'), RunMode.DefaultMode) + + def test_missing_run_in_mode_is_default_mode(self): + self.assertIs(parse_flow_with_run_in_mode(None), RunMode.DefaultMode) + + def test_unknown_run_in_mode_falls_back_to_default_mode_instead_of_crashing(self): + with self.assertLogs('flow_parser.parse', level='WARNING'): + self.assertIs(parse_flow_with_run_in_mode('SomeFutureMode'), RunMode.DefaultMode) + + def test_user_mode_subflow_inherits_the_parent_mode(self): + # Like DefaultMode, UserMode must not override a system mode that a parent flow is already running in + self.assertIs(util.get_effective_run_mode(RunMode.SystemModeWithoutSharing, RunMode.UserMode), + RunMode.SystemModeWithoutSharing) + self.assertIs(util.get_effective_run_mode(None, RunMode.UserMode), RunMode.UserMode) + + +if __name__ == '__main__': + unittest.main() diff --git a/packages/code-analyzer-flow-engine/src/messages.ts b/packages/code-analyzer-flow-engine/src/messages.ts index 0dba4231..549c5b8f 100644 --- a/packages/code-analyzer-flow-engine/src/messages.ts +++ b/packages/code-analyzer-flow-engine/src/messages.ts @@ -20,6 +20,12 @@ const MESSAGE_CATALOG: {[key: string]: string} = { ResultsFileNotValidJson: `Unexpected error: Results file contents are not a valid JSON: %s`, + ResultsFileNotProduced: + `Unexpected error: The Flow Scanner did not produce a results file, which usually means it failed to parse one or more flows. See the log file for details: %s`, + + ResultsFileNotReadable: + `Unexpected error: The Flow Scanner results file '%s' could not be read: %s`, + CouldNotParseExecutionResults: `Unexpected error: Could not parse results from %s`, diff --git a/packages/code-analyzer-flow-engine/src/python/FlowScannerCommandWrapper.ts b/packages/code-analyzer-flow-engine/src/python/FlowScannerCommandWrapper.ts index 257de506..548501a4 100644 --- a/packages/code-analyzer-flow-engine/src/python/FlowScannerCommandWrapper.ts +++ b/packages/code-analyzer-flow-engine/src/python/FlowScannerCommandWrapper.ts @@ -101,7 +101,18 @@ export class RunTimeFlowScannerCommandWrapper implements FlowScannerCommandWrapp await this.pythonCommandExecutor.exec(pythonArgs, processStdout); - const outputFileContents: string = await fs.promises.readFile(flowScannerResultsFile, 'utf-8'); + let outputFileContents: string; + try { + outputFileContents = await fs.promises.readFile(flowScannerResultsFile, 'utf-8'); + } catch (err) { + // Only a missing file means the scanner failed to produce results. Any other failure (permissions, the + // path being a directory, etc.) is a different problem, so we keep the original error as the cause. + if ((err as NodeJS.ErrnoException)?.code === 'ENOENT') { + throw new Error(getMessage('ResultsFileNotProduced', absLogFilePath)); + } + throw new Error(getMessage('ResultsFileNotReadable', flowScannerResultsFile, (err as Error)?.message ?? String(err)), + {cause: err}); + } let parsedResults: object; try { diff --git a/packages/code-analyzer-flow-engine/test/python/FlowScannerCommandWrapper.test.ts b/packages/code-analyzer-flow-engine/test/python/FlowScannerCommandWrapper.test.ts index 48bda38f..e8c24712 100644 --- a/packages/code-analyzer-flow-engine/test/python/FlowScannerCommandWrapper.test.ts +++ b/packages/code-analyzer-flow-engine/test/python/FlowScannerCommandWrapper.test.ts @@ -184,6 +184,25 @@ describe('FlowScannerCommandWrapper implementations', () => { .rejects .toThrow(expectedMessage); }); + + it.each([ + {code: 'ENOENT', expectedMessage: 'did not produce a results file'}, + {code: 'EACCES', expectedMessage: 'could not be read'}, + {code: 'EISDIR', expectedMessage: 'could not be read'} + ])('When reading the results file fails with $code, the error message reflects the real problem', async ({code, expectedMessage}) => { + jest.spyOn(PythonCommandExecutor.prototype, 'exec').mockImplementation(async (_args, _processStdout) => { + return Promise.resolve(); + }); + const fsError: NodeJS.ErrnoException = Object.assign(new Error(`${code}: simulated failure`), {code}); + jest.spyOn(fs.promises, 'readFile').mockRejectedValue(fsError); + + const wrapper: RunTimeFlowScannerCommandWrapper = new RunTimeFlowScannerCommandWrapper(PYTHON_COMMAND); + const promise = wrapper.runFlowScannerRules(workingFolder, [PATH_TO_EXAMPLE1, PATH_TO_EXAMPLE2], [PATH_TO_EXAMPLE1, PATH_TO_EXAMPLE2], tempLogFile, [], (_num: number) => {}); + await expect(promise).rejects.toThrow(expectedMessage); + if (code !== 'ENOENT') { + await expect(promise).rejects.toMatchObject({cause: fsError}); + } + }); }); }); });