API uses "bare" exceptions in multiple places

@adamrehn

Per our conversation with @TBBle under #65, looks like there are several bare exceptions in current API.
This is considered a bad practice and non-Pythonic way of catching exceptions - reference1, reference2
It can potentially suppress unwanted exceptions and hide bugs. Something that we don't really want.

Fixing it right now is a bit risky, especially at this stage of the project. It would require some retesting and probably adding more code for edge cases. However, it would be a good change after all. I can try fixing it.

Let me know what do you think!

Example:

ue4cli/ue4cli/UnrealManagerBase.py

Lines 167 to 178 in fed71c1

    
           	def getDescriptor(self, dir): 
        
           		""" 
        
           		Detects the descriptor file for either an Unreal project or an Unreal plugin in the specified directory 
        
           		""" 
        
           		try: 
        
           			return self.getProjectDescriptor(dir) 
        
           		except: 
        
           			try: 
        
           				return self.getPluginDescriptor(dir) 
        
           			except: 
        
           				raise UnrealManagerException('could not detect an Unreal project or plugin in the directory "{}"'.format(dir))

For reference, grep shows at least 7 usages of bare exception:

╰─$ grep -Rno "except:" . 
./ue4cli/UE4BuildInterrogator.py:192:except:
./ue4cli/UnrealManagerBase.py:52:except:
./ue4cli/UnrealManagerBase.py:173:except:
./ue4cli/UnrealManagerBase.py:176:except:
./ue4cli/UnrealManagerWindows.py:9:except:
./ue4cli/UnrealManagerWindows.py:42:except:
./ue4cli/UnrealManagerWindows.py:95:except:

Similar case appears here. Failure from try-finally block is never handled.

ue4cli/ue4cli/UE4BuildInterrogator.py

Lines 165 to 176 in fed71c1

    
           # Invoke UnrealBuildTool in JSON export mode (make sure we specify gathering mode, since this is a prerequisite of JSON export) 
        
           # (Ensure we always perform sentinel file cleanup even when errors occur) 
        
           try: 
        
           	args = ['-Mode=JsonExport', '-OutputFile=' +jsonFile ] if (self.engineVersion['MajorVersion'] >= 5 or self.engineVersion['MinorVersion'] >= 22) else ['-gather', '-jsonexport=' + jsonFile, '-SkipBuild'] 
        
           	if self.engineVersion['MajorVersion'] >= 5: 
        
           		self.runUBTFunc('UnrealEditor', platformIdentifier, configuration, args) 
        
           	else: 
        
           		self.runUBTFunc('UE4Editor', platformIdentifier, configuration, args) 
        
           finally: 
        
           	if renameSentinel == True: 
        
           		shutil.move(sentinelBackup, sentinelFile)

Similar case appears here. Failure from try-finally block is never handled.

ue4cli/ue4cli/UE4BuildInterrogator.py

Lines 165 to 176 in fed71c1

# Invoke UnrealBuildTool in JSON export mode (make sure we specify gathering mode, since this is a prerequisite of JSON export)

# (Ensure we always perform sentinel file cleanup even when errors occur)

try:

args = ['-Mode=JsonExport', '-OutputFile=' +jsonFile ] if (self.engineVersion['MajorVersion'] >= 5 or self.engineVersion['MinorVersion'] >= 22) else ['-gather', '-jsonexport=' + jsonFile, '-SkipBuild']

if self.engineVersion['MajorVersion'] >= 5:

self.runUBTFunc('UnrealEditor', platformIdentifier, configuration, args)

else:

self.runUBTFunc('UE4Editor', platformIdentifier, configuration, args)

finally:

if renameSentinel == True:

shutil.move(sentinelBackup, sentinelFile)

This one's a bit different, it's just letting any exceptions propagate up (for better or worse but from another function in the same module) but ensuring that clean-up happens either way.

In this case, particularly, someone hitting Control-C into UBT and triggering a KeyboardInterrupt for us should not prevent moving the backup back into place if necessary, but probably doesn't need to be otherwise handled specially here; i.e. this is just an open-coded context manager.

Navigation Menu

Repository navigation

API uses "bare" exceptions in multiple places #67

Metadata

Assignees

Labels

Projects

Milestone

Relationships

Development

	def getDescriptor(self, dir):
	"""
	Detects the descriptor file for either an Unreal project or an Unreal plugin in the specified directory
	"""
	try:
	return self.getProjectDescriptor(dir)
	except:
	try:
	return self.getPluginDescriptor(dir)
	except:
	raise UnrealManagerException('could not detect an Unreal project or plugin in the directory "{}"'.format(dir))

	# Invoke UnrealBuildTool in JSON export mode (make sure we specify gathering mode, since this is a prerequisite of JSON export)
	# (Ensure we always perform sentinel file cleanup even when errors occur)
	try:
	args = ['-Mode=JsonExport', '-OutputFile=' +jsonFile ] if (self.engineVersion['MajorVersion'] >= 5 or self.engineVersion['MinorVersion'] >= 22) else ['-gather', '-jsonexport=' + jsonFile, '-SkipBuild']
	if self.engineVersion['MajorVersion'] >= 5:
	self.runUBTFunc('UnrealEditor', platformIdentifier, configuration, args)
	else:
	self.runUBTFunc('UE4Editor', platformIdentifier, configuration, args)
	finally:
	if renameSentinel == True:
	shutil.move(sentinelBackup, sentinelFile)

API uses "bare" exceptions in multiple places #67

Description

Activity

sleeptightAnsiC commented on Mar 23, 2024

TBBle commented on Mar 24, 2024

Metadata

Metadata

Assignees

Labels

Projects

Milestone

Relationships

Development

Issue actions