Skip to content

Commit b07c433

Browse files
MarshalXMarshalX
authored
CM-48357 - Fix SCA restore error handling (#309)
1 parent 4e1f7e0 commit b07c433

10 files changed

Lines changed: 55 additions & 74 deletions

‎cycode/cli/apps/scan/code_scanner.py‎

Lines changed: 10 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -683,8 +683,8 @@ def try_get_git_remote_url(path: str) -> Optional[str]:
683683
remote_url = git_proxy.get_repo(path).remotes[0].config_reader.get('url')
684684
logger.debug('Found Git remote URL, %s', {'remote_url': remote_url, 'path': path})
685685
return remote_url
686-
except Exception as e:
687-
logger.debug('Failed to get Git remote URL', exc_info=e)
686+
except Exception:
687+
logger.debug('Failed to get Git remote URL. Probably not a Git repository')
688688
return None
689689

690690

@@ -706,7 +706,9 @@ def _get_plastic_repository_name(path: str) -> Optional[str]:
706706
f'--fieldseparator={consts.PLASTIC_VCS_DATA_SEPARATOR}',
707707
]
708708

709-
status = shell(command=command, timeout=consts.PLASTIC_VSC_CLI_TIMEOUT, working_directory=path)
709+
status = shell(
710+
command=command, timeout=consts.PLASTIC_VSC_CLI_TIMEOUT, working_directory=path, silent_exc_info=True
711+
)
710712
if not status:
711713
logger.debug('Failed to get Plastic repository name (command failed)')
712714
return None
@@ -717,8 +719,8 @@ def _get_plastic_repository_name(path: str) -> Optional[str]:
717719
return None
718720

719721
return status_parts[2].strip()
720-
except Exception as e:
721-
logger.debug('Failed to get Plastic repository name', exc_info=e)
722+
except Exception:
723+
logger.debug('Failed to get Plastic repository name. Probably not a Plastic repository')
722724
return None
723725

724726

@@ -738,7 +740,9 @@ def _get_plastic_repository_list(working_dir: Optional[str] = None) -> dict[str,
738740
try:
739741
command = ['cm', 'repo', 'ls', f'--format={{repname}}{consts.PLASTIC_VCS_DATA_SEPARATOR}{{repguid}}']
740742

741-
status = shell(command=command, timeout=consts.PLASTIC_VSC_CLI_TIMEOUT, working_directory=working_dir)
743+
status = shell(
744+
command=command, timeout=consts.PLASTIC_VSC_CLI_TIMEOUT, working_directory=working_dir, silent_exc_info=True
745+
)
742746
if not status:
743747
logger.debug('Failed to get Plastic repository list (command failed)')
744748
return repo_name_to_guid

‎cycode/cli/files_collector/sca/base_restore_dependencies.py‎

Lines changed: 23 additions & 28 deletions
Original file line numberDiff line numberDiff line change
@@ -1,9 +1,9 @@
1+
import os
12
from abc import ABC, abstractmethod
23
from typing import Optional
34

45
import typer
56

6-
from cycode.cli.logger import logger
77
from cycode.cli.models import Document
88
from cycode.cli.utils.path_utils import get_file_content, get_file_dir, get_path_from_context, join_paths
99
from cycode.cli.utils.shell_executor import shell
@@ -15,30 +15,27 @@ def build_dep_tree_path(path: str, generated_file_name: str) -> str:
1515

1616
def execute_commands(
1717
commands: list[list[str]],
18-
file_name: str,
19-
command_timeout: int,
20-
dependencies_file_name: Optional[str] = None,
18+
timeout: int,
19+
output_file_path: Optional[str] = None,
2120
working_directory: Optional[str] = None,
2221
) -> Optional[str]:
2322
try:
24-
all_dependencies = []
23+
outputs = []
2524

26-
# Run all commands and collect outputs
2725
for command in commands:
28-
dependencies = shell(command=command, timeout=command_timeout, working_directory=working_directory)
29-
all_dependencies.append(dependencies) # Collect each command's output
26+
command_output = shell(command=command, timeout=timeout, working_directory=working_directory)
27+
if command_output:
28+
outputs.append(command_output)
3029

31-
dependencies = '\n'.join(all_dependencies)
30+
joined_output = '\n'.join(outputs)
3231

33-
# Write all collected outputs to the file if dependencies_file_name is provided
34-
if dependencies_file_name:
35-
with open(dependencies_file_name, 'w') as output_file: # Open once in 'w' mode to start fresh
36-
output_file.writelines(dependencies)
37-
except Exception as e:
38-
logger.debug('Failed to restore dependencies via shell command, %s', {'filename': file_name}, exc_info=e)
32+
if output_file_path:
33+
with open(output_file_path, 'w', encoding='UTF-8') as output_file:
34+
output_file.writelines(joined_output)
35+
except Exception:
3936
return None
4037

41-
return dependencies
38+
return joined_output
4239

4340

4441
class BaseRestoreDependencies(ABC):
@@ -64,27 +61,25 @@ def try_restore_dependencies(self, document: Document) -> Optional[Document]:
6461
relative_restore_file_path = build_dep_tree_path(document.path, self.get_lock_file_name())
6562
working_directory_path = self.get_working_directory(document)
6663

67-
if self.verify_restore_file_already_exist(restore_file_path):
68-
restore_file_content = get_file_content(restore_file_path)
69-
else:
70-
output_file_path = restore_file_path if self.create_output_file_manually else None
71-
execute_commands(
64+
if not self.verify_restore_file_already_exist(restore_file_path):
65+
output = execute_commands(
7266
self.get_commands(manifest_file_path),
73-
manifest_file_path,
7467
self.command_timeout,
75-
output_file_path,
76-
working_directory_path,
68+
output_file_path=restore_file_path if self.create_output_file_manually else None,
69+
working_directory=working_directory_path,
7770
)
78-
restore_file_content = get_file_content(restore_file_path)
71+
if output is None: # one of the commands failed
72+
return None
7973

74+
restore_file_content = get_file_content(restore_file_path)
8075
return Document(relative_restore_file_path, restore_file_content, self.is_git_diff)
8176

8277
def get_working_directory(self, document: Document) -> Optional[str]:
8378
return None
8479

85-
@abstractmethod
86-
def verify_restore_file_already_exist(self, restore_file_path: str) -> bool:
87-
pass
80+
@staticmethod
81+
def verify_restore_file_already_exist(restore_file_path: str) -> bool:
82+
return os.path.isfile(restore_file_path)
8883

8984
@abstractmethod
9085
def is_project(self, document: Document) -> bool:

‎cycode/cli/files_collector/sca/go/restore_go_dependencies.py‎

Lines changed: 0 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -44,8 +44,5 @@ def get_commands(self, manifest_file_path: str) -> list[list[str]]:
4444
def get_lock_file_name(self) -> str:
4545
return GO_RESTORE_FILE_NAME
4646

47-
def verify_restore_file_already_exist(self, restore_file_path: str) -> bool:
48-
return os.path.isfile(restore_file_path)
49-
5047
def get_working_directory(self, document: Document) -> Optional[str]:
5148
return os.path.dirname(document.absolute_path)

‎cycode/cli/files_collector/sca/maven/restore_gradle_dependencies.py‎

Lines changed: 4 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -42,22 +42,19 @@ def get_commands(self, manifest_file_path: str) -> list[list[str]]:
4242
def get_lock_file_name(self) -> str:
4343
return BUILD_GRADLE_DEP_TREE_FILE_NAME
4444

45-
def verify_restore_file_already_exist(self, restore_file_path: str) -> bool:
46-
return os.path.isfile(restore_file_path)
47-
4845
def get_working_directory(self, document: Document) -> Optional[str]:
4946
return get_path_from_context(self.ctx) if self.is_gradle_sub_projects() else None
5047

5148
def get_all_projects(self) -> set[str]:
52-
projects_output = shell(
49+
output = shell(
5350
command=BUILD_GRADLE_ALL_PROJECTS_COMMAND,
5451
timeout=BUILD_GRADLE_ALL_PROJECTS_TIMEOUT,
5552
working_directory=get_path_from_context(self.ctx),
5653
)
54+
if not output:
55+
return set()
5756

58-
projects = re.findall(ALL_PROJECTS_REGEX, projects_output)
59-
60-
return set(projects)
57+
return set(re.findall(ALL_PROJECTS_REGEX, output))
6158

6259
def get_commands_for_sub_projects(self, manifest_file_path: str) -> list[list[str]]:
6360
project_name = os.path.basename(os.path.dirname(manifest_file_path))

‎cycode/cli/files_collector/sca/maven/restore_maven_dependencies.py‎

Lines changed: 12 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,3 @@
1-
import os
21
from os import path
32
from typing import Optional
43

@@ -30,9 +29,6 @@ def get_commands(self, manifest_file_path: str) -> list[list[str]]:
3029
def get_lock_file_name(self) -> str:
3130
return join_paths('target', MAVEN_CYCLONE_DEP_TREE_FILE_NAME)
3231

33-
def verify_restore_file_already_exist(self, restore_file_path: str) -> bool:
34-
return os.path.isfile(restore_file_path)
35-
3632
def try_restore_dependencies(self, document: Document) -> Optional[Document]:
3733
restore_dependencies_document = super().try_restore_dependencies(document)
3834
manifest_file_path = self.get_manifest_file_path(document)
@@ -51,8 +47,8 @@ def restore_from_secondary_command(
5147
self, document: Document, manifest_file_path: str, restore_dependencies_document: Optional[Document]
5248
) -> Optional[Document]:
5349
# TODO(MarshalX): does it even work? Ignored restore_dependencies_document arg
54-
secondary_restore_command = create_secondary_restore_command(manifest_file_path)
55-
backup_restore_content = execute_commands(secondary_restore_command, manifest_file_path, self.command_timeout)
50+
secondary_restore_command = create_secondary_restore_commands(manifest_file_path)
51+
backup_restore_content = execute_commands(secondary_restore_command, self.command_timeout)
5652
restore_dependencies_document = Document(
5753
build_dep_tree_path(document.path, MAVEN_DEP_TREE_FILE_NAME), backup_restore_content, self.is_git_diff
5854
)
@@ -64,13 +60,15 @@ def restore_from_secondary_command(
6460
return restore_dependencies
6561

6662

67-
def create_secondary_restore_command(manifest_file_path: str) -> list[str]:
63+
def create_secondary_restore_commands(manifest_file_path: str) -> list[list[str]]:
6864
return [
69-
'mvn',
70-
'dependency:tree',
71-
'-B',
72-
'-DoutputType=text',
73-
'-f',
74-
manifest_file_path,
75-
f'-DoutputFile={MAVEN_DEP_TREE_FILE_NAME}',
65+
[
66+
'mvn',
67+
'dependency:tree',
68+
'-B',
69+
'-DoutputType=text',
70+
'-f',
71+
manifest_file_path,
72+
f'-DoutputFile={MAVEN_DEP_TREE_FILE_NAME}',
73+
]
7674
]

‎cycode/cli/files_collector/sca/npm/restore_npm_dependencies.py‎

Lines changed: 0 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -33,9 +33,6 @@ def get_commands(self, manifest_file_path: str) -> list[list[str]]:
3333
def get_lock_file_name(self) -> str:
3434
return NPM_LOCK_FILE_NAME
3535

36-
def verify_restore_file_already_exist(self, restore_file_path: str) -> bool:
37-
return os.path.isfile(restore_file_path)
38-
3936
@staticmethod
4037
def prepare_manifest_file_path_for_command(manifest_file_path: str) -> str:
4138
return manifest_file_path.replace(os.sep + NPM_MANIFEST_FILE_NAME, '')

‎cycode/cli/files_collector/sca/nuget/restore_nuget_dependencies.py‎

Lines changed: 0 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,3 @@
1-
import os
2-
31
import typer
42

53
from cycode.cli.files_collector.sca.base_restore_dependencies import BaseRestoreDependencies
@@ -21,6 +19,3 @@ def get_commands(self, manifest_file_path: str) -> list[list[str]]:
2119

2220
def get_lock_file_name(self) -> str:
2321
return NUGET_LOCK_FILE_NAME
24-
25-
def verify_restore_file_already_exist(self, restore_file_path: str) -> bool:
26-
return os.path.isfile(restore_file_path)

‎cycode/cli/files_collector/sca/ruby/restore_ruby_dependencies.py‎

Lines changed: 0 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -18,8 +18,5 @@ def get_commands(self, manifest_file_path: str) -> list[list[str]]:
1818
def get_lock_file_name(self) -> str:
1919
return RUBY_LOCK_FILE_NAME
2020

21-
def verify_restore_file_already_exist(self, restore_file_path: str) -> bool:
22-
return os.path.isfile(restore_file_path)
23-
2421
def get_working_directory(self, document: Document) -> Optional[str]:
2522
return os.path.dirname(document.absolute_path)

‎cycode/cli/files_collector/sca/sbt/restore_sbt_dependencies.py‎

Lines changed: 0 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -18,8 +18,5 @@ def get_commands(self, manifest_file_path: str) -> list[list[str]]:
1818
def get_lock_file_name(self) -> str:
1919
return SBT_LOCK_FILE_NAME
2020

21-
def verify_restore_file_already_exist(self, restore_file_path: str) -> bool:
22-
return os.path.isfile(restore_file_path)
23-
2421
def get_working_directory(self, document: Document) -> Optional[str]:
2522
return os.path.dirname(document.absolute_path)

‎cycode/cli/utils/shell_executor.py‎

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,7 @@ def shell(
1616
command: Union[str, list[str]],
1717
timeout: int = _SUBPROCESS_DEFAULT_TIMEOUT_SEC,
1818
working_directory: Optional[str] = None,
19+
silent_exc_info: bool = False,
1920
) -> Optional[str]:
2021
logger.debug('Executing shell command: %s', command)
2122

@@ -27,12 +28,15 @@ def shell(
2728

2829
return result.stdout.decode('UTF-8').strip()
2930
except subprocess.CalledProcessError as e:
30-
logger.debug('Error occurred while running shell command', exc_info=e)
31+
if not silent_exc_info:
32+
logger.debug('Error occurred while running shell command', exc_info=e)
3133
except subprocess.TimeoutExpired as e:
3234
logger.debug('Command timed out', exc_info=e)
3335
raise typer.Abort(f'Command "{command}" timed out') from e
3436
except Exception as e:
35-
logger.debug('Unhandled exception occurred while running shell command', exc_info=e)
37+
if not silent_exc_info:
38+
logger.debug('Unhandled exception occurred while running shell command', exc_info=e)
39+
3640
raise click.ClickException(f'Unhandled exception: {e}') from e
3741

3842
return None

0 commit comments

Comments
 (0)