Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

More specific exception types and no more silent errors #868

Open
wants to merge 4 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 2 additions & 1 deletion developer/bake_config.py
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,7 @@
from tank.descriptor import create_descriptor, is_descriptor_version_missing
from tank.descriptor.errors import TankDescriptorError
from tank.bootstrap import constants as bootstrap_constants
from shotgun_api3 import ShotgunError
import functools

from utils import (
Expand Down Expand Up @@ -293,7 +294,7 @@ def main():
# make sure we are properly connected
try:
sg_connection.find_one("HumanUser", [])
except Exception as e:
except ShotgunError as e:
logger.error("Could not communicate with Flow Production Tracking: %s" % e)
return 3

Expand Down
3 changes: 2 additions & 1 deletion hooks/get_current_login.py
Original file line number Diff line number Diff line change
Expand Up @@ -54,5 +54,6 @@ def execute(self, **kwargs):

pwd_entry = pwd.getpwuid(os.geteuid())
return pwd_entry[0]
except:
except Exception:
self.logger.debug("GetCurrentLogin hook failed.", exc_info=True)
return None
3 changes: 2 additions & 1 deletion python/tank/api.py
Original file line number Diff line number Diff line change
Expand Up @@ -296,7 +296,8 @@
data = str(data.get("documentation_url"))
if data == "":
data = None
except Exception:
except Exception as e:
log.debug("Cant get documentation url: %s" % e)

Check warning on line 300 in python/tank/api.py

View check run for this annotation

Codecov / codecov/patch

python/tank/api.py#L299-L300

Added lines #L299 - L300 were not covered by tests
data = None

return data
Expand Down
10 changes: 6 additions & 4 deletions python/tank/authentication/interactive_authentication.py
Original file line number Diff line number Diff line change
Expand Up @@ -41,13 +41,14 @@
# something usually done by the Toolkit. The worry is that the import may fail
# in the context of a DCC, but occur too early for the Toolkit logging to be
# fully in place to record it.
logger = LogManager.get_logger(__name__)

try:
from .ui.qt_abstraction import QtGui
except Exception:
except ImportError as e:
logger.debug("Cant import QtGui: %s" % e)

Check warning on line 49 in python/tank/authentication/interactive_authentication.py

View check run for this annotation

Codecov / codecov/patch

python/tank/authentication/interactive_authentication.py#L48-L49

Added lines #L48 - L49 were not covered by tests
QtGui = None

logger = LogManager.get_logger(__name__)


###############################################################################################
# internal classes and methods
Expand All @@ -68,7 +69,8 @@

pwd_entry = pwd.getpwuid(os.geteuid())
return pwd_entry[0]
except:
except Exception as e:
logger.debug("Cant get current user: %s" % e)

Check warning on line 73 in python/tank/authentication/interactive_authentication.py

View check run for this annotation

Codecov / codecov/patch

python/tank/authentication/interactive_authentication.py#L72-L73

Added lines #L72 - L73 were not covered by tests
return None


Expand Down
3 changes: 2 additions & 1 deletion python/tank/authentication/invoker.py
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,8 @@
# fully in place to record it.
try:
from .ui.qt_abstraction import QtCore, QtGui
except Exception:
except ImportError as e:
logger.debug("Cant import QtCore/QtGui: %s" % e)

Check warning on line 34 in python/tank/authentication/invoker.py

View check run for this annotation

Codecov / codecov/patch

python/tank/authentication/invoker.py#L33-L34

Added lines #L33 - L34 were not covered by tests
QtCore, QtGui = None, None


Expand Down
2 changes: 1 addition & 1 deletion python/tank/authentication/session_cache.py
Original file line number Diff line number Diff line change
Expand Up @@ -654,6 +654,6 @@
# If the error message is empty, like httplib.HTTPException, convert
# the class name to a string
if len(str(e)) == 0:
raise Exception("Unknown error: %s" % type(e).__name__)
raise type(e)("Unknown error: %s" % type(e).__name__)

Check warning on line 657 in python/tank/authentication/session_cache.py

View check run for this annotation

Codecov / codecov/patch

python/tank/authentication/session_cache.py#L657

Added line #L657 was not covered by tests
else:
raise
Original file line number Diff line number Diff line change
Expand Up @@ -13,12 +13,10 @@

from __future__ import print_function

# pylint: disable=import-error
from ...ui.qt_abstraction import QtCore, QtGui

# No point in proceeding if QtGui is None.
if QtGui is None:
raise ImportError("Unable to import QtGui")
try:
from ...ui.qt_abstraction import QtCore, QtGui
except ImportError:
raise

Check warning on line 19 in python/tank/authentication/sso_saml2/core/username_password_dialog.py

View check run for this annotation

Codecov / codecov/patch

python/tank/authentication/sso_saml2/core/username_password_dialog.py#L18-L19

Added lines #L18 - L19 were not covered by tests


class UsernamePasswordDialog(QtGui.QDialog):
Expand Down
3 changes: 2 additions & 1 deletion python/tank/authentication/ui_authentication.py
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,8 @@
# fully in place to record it.
try:
from .login_dialog import LoginDialog
except Exception:
except ImportError as e:
logger.debug("Cant import LoginDialog: %s" % e)

Check warning on line 36 in python/tank/authentication/ui_authentication.py

View check run for this annotation

Codecov / codecov/patch

python/tank/authentication/ui_authentication.py#L35-L36

Added lines #L35 - L36 were not covered by tests
LoginDialog = None


Expand Down
13 changes: 7 additions & 6 deletions python/tank/bootstrap/async_bootstrap.py
Original file line number Diff line number Diff line change
Expand Up @@ -11,12 +11,13 @@
# Import Qt without having to worry about the version to use.
from ..util.qt_importer import QtImporter

importer = QtImporter()
QtCore = importer.QtCore
QtGui = importer.QtGui
if QtCore is None:
# Raise an exception when Qt is not available.
raise ImportError
try:
importer = QtImporter()
except ImportError:
raise

Check warning on line 17 in python/tank/bootstrap/async_bootstrap.py

View check run for this annotation

Codecov / codecov/patch

python/tank/bootstrap/async_bootstrap.py#L14-L17

Added lines #L14 - L17 were not covered by tests
else:
QtCore = importer.QtCore
QtGui = importer.QtGui

Check warning on line 20 in python/tank/bootstrap/async_bootstrap.py

View check run for this annotation

Codecov / codecov/patch

python/tank/bootstrap/async_bootstrap.py#L19-L20

Added lines #L19 - L20 were not covered by tests


class AsyncBootstrapWrapper(QtCore.QObject):
Expand Down
2 changes: 1 addition & 1 deletion python/tank/commands/cache_yaml.py
Original file line number Diff line number Diff line change
Expand Up @@ -86,7 +86,7 @@

try:
fh = open(pickle_path, "wb")
except Exception as e:
except OSError as e:

Check warning on line 89 in python/tank/commands/cache_yaml.py

View check run for this annotation

Codecov / codecov/patch

python/tank/commands/cache_yaml.py#L89

Added line #L89 was not covered by tests
raise TankError("Unable to open '%s' for writing: %s" % (pickle_path, e))

try:
Expand Down
2 changes: 1 addition & 1 deletion python/tank/commands/clone_configuration.py
Original file line number Diff line number Diff line change
Expand Up @@ -280,7 +280,7 @@
fh.write("# End of file.\n")
fh.close()

except Exception as e:
except OSError as e:

Check warning on line 283 in python/tank/commands/clone_configuration.py

View check run for this annotation

Codecov / codecov/patch

python/tank/commands/clone_configuration.py#L283

Added line #L283 was not covered by tests
raise TankError("Could not create file system structure: %s" % e)

# lastly, update the pipeline_configuration.yml file
Expand Down
2 changes: 1 addition & 1 deletion python/tank/commands/console_utils.py
Original file line number Diff line number Diff line change
Expand Up @@ -228,7 +228,7 @@
param_name,
answer,
)
except Exception as e:
except TankError as e:

Check warning on line 231 in python/tank/commands/console_utils.py

View check run for this annotation

Codecov / codecov/patch

python/tank/commands/console_utils.py#L231

Added line #L231 was not covered by tests
log.error("Validation failed: %s" % e)
else:
input_valid = True
Expand Down
4 changes: 2 additions & 2 deletions python/tank/commands/dump_config.py
Original file line number Diff line number Diff line change
Expand Up @@ -213,7 +213,7 @@
log.info(
"Environment written to: %s" % (os.path.abspath(params["file"]))
)
except Exception as e:
except TankError as e:

Check warning on line 216 in python/tank/commands/dump_config.py

View check run for this annotation

Codecov / codecov/patch

python/tank/commands/dump_config.py#L216

Added line #L216 was not covered by tests
import traceback

traceback.print_exc()
Expand Down Expand Up @@ -244,7 +244,7 @@
)
try:
fh = open(path, "w")
except Exception as e:
except OSError as e:

Check warning on line 247 in python/tank/commands/dump_config.py

View check run for this annotation

Codecov / codecov/patch

python/tank/commands/dump_config.py#L247

Added line #L247 was not covered by tests
raise TankError(
"Unable to open file: %s\n" " Error reported: %s" % (path, e)
)
Expand Down
4 changes: 2 additions & 2 deletions python/tank/commands/install.py
Original file line number Diff line number Diff line change
Expand Up @@ -226,7 +226,7 @@
env_name, writable=True
)
env.set_yaml_preserve_mode(preserve_yaml)
except Exception as e:
except TankError as e:

Check warning on line 229 in python/tank/commands/install.py

View check run for this annotation

Codecov / codecov/patch

python/tank/commands/install.py#L229

Added line #L229 was not covered by tests
raise TankError(
"Environment '%s' could not be loaded! Error reported: %s"
% (env_name, e)
Expand Down Expand Up @@ -528,7 +528,7 @@
env_name, writable=True
)
env.set_yaml_preserve_mode(preserve_yaml)
except Exception as e:
except TankError as e:

Check warning on line 531 in python/tank/commands/install.py

View check run for this annotation

Codecov / codecov/patch

python/tank/commands/install.py#L531

Added line #L531 was not covered by tests
raise TankError(
"Environment '%s' could not be loaded! Error reported: %s"
% (env_name, e)
Expand Down
10 changes: 6 additions & 4 deletions python/tank/commands/misc.py
Original file line number Diff line number Diff line change
Expand Up @@ -70,8 +70,10 @@
log.debug("Deleting cache file %s..." % full_path)
try:
os.remove(full_path)
except:
log.warning("Could not delete cache file '%s'!" % full_path)
except OSError as e:
log.warning(

Check warning on line 74 in python/tank/commands/misc.py

View check run for this annotation

Codecov / codecov/patch

python/tank/commands/misc.py#L73-L74

Added lines #L73 - L74 were not covered by tests
"Could not delete cache file '%s'! // %s" % (full_path, e)
)

log.info("The PTR menu cache has been cleared.")

Expand Down Expand Up @@ -137,7 +139,7 @@
readline.parse_and_bind("bind ^I rl_complete")
else:
readline.parse_and_bind("tab: complete")
except:
pass
except Exception as e:
msg.append(e)

Check warning on line 143 in python/tank/commands/misc.py

View check run for this annotation

Codecov / codecov/patch

python/tank/commands/misc.py#L142-L143

Added lines #L142 - L143 were not covered by tests

code.interact(banner="\n".join(msg), local=tk_locals)
6 changes: 3 additions & 3 deletions python/tank/commands/move_pc.py
Original file line number Diff line number Diff line change
Expand Up @@ -48,7 +48,7 @@
log.debug("Removing %s..." % full_path)
try:
os.remove(full_path)
except Exception as e:
except OSError as e:

Check warning on line 51 in python/tank/commands/move_pc.py

View check run for this annotation

Codecov / codecov/patch

python/tank/commands/move_pc.py#L51

Added line #L51 was not covered by tests
log.warning(
"Could not delete file %s. Error Reported: %s"
% (full_path, e)
Expand All @@ -68,7 +68,7 @@
log.debug("Deleting folder %s..." % full_path)
try:
os.rmdir(full_path)
except Exception as e:
except OSError as e:

Check warning on line 71 in python/tank/commands/move_pc.py

View check run for this annotation

Codecov / codecov/patch

python/tank/commands/move_pc.py#L71

Added line #L71 was not covered by tests
log.warning(
"Could not remove folder %s. Error Reported: %s"
% (full_path, e)
Expand Down Expand Up @@ -272,7 +272,7 @@
fh.write("# End of file.\n")
fh.close()

except Exception as e:
except OSError as e:

Check warning on line 275 in python/tank/commands/move_pc.py

View check run for this annotation

Codecov / codecov/patch

python/tank/commands/move_pc.py#L275

Added line #L275 was not covered by tests
raise TankError(
"Could not copy configuration! This may be because of system "
"permissions or system setup. This configuration will "
Expand Down
6 changes: 3 additions & 3 deletions python/tank/commands/push_pc.py
Original file line number Diff line number Diff line change
Expand Up @@ -137,8 +137,8 @@
raise TankError("Aborted by user.")
try:
target_pc_id = int(answer)
except:
raise TankError("Please enter a number!")
except ValueError as e:
raise TankError("Please enter a number! // %s" % e)

Check warning on line 141 in python/tank/commands/push_pc.py

View check run for this annotation

Codecov / codecov/patch

python/tank/commands/push_pc.py#L140-L141

Added lines #L140 - L141 were not covered by tests

self._run(
log,
Expand Down Expand Up @@ -230,7 +230,7 @@
for env_name in self.tk.pipeline_configuration.get_environments():
try:
env = self.tk.pipeline_configuration.get_environment(env_name)
except Exception as e:
except TankError as e:

Check warning on line 233 in python/tank/commands/push_pc.py

View check run for this annotation

Codecov / codecov/patch

python/tank/commands/push_pc.py#L233

Added line #L233 was not covered by tests
raise TankError(
"Failed to load environment %s,"
" run 'tank validate' for more details, got error: %s"
Expand Down
4 changes: 2 additions & 2 deletions python/tank/commands/setup_project.py
Original file line number Diff line number Diff line change
Expand Up @@ -548,8 +548,8 @@
raise TankError("Aborted by user.")
try:
project_id = int(answer)
except:
raise TankError("Please enter a number!")
except ValueError as e:
raise TankError("Please enter a number! // %s" % e)

Check warning on line 552 in python/tank/commands/setup_project.py

View check run for this annotation

Codecov / codecov/patch

python/tank/commands/setup_project.py#L551-L552

Added lines #L551 - L552 were not covered by tests

if project_id not in [x["id"] for x in projs]:
raise TankError("Id %d was not found in the list of projects!" % project_id)
Expand Down
2 changes: 1 addition & 1 deletion python/tank/commands/switch.py
Original file line number Diff line number Diff line change
Expand Up @@ -143,7 +143,7 @@
env_name, writable=True
)
env.set_yaml_preserve_mode(preserve_yaml)
except Exception as e:
except TankError as e:

Check warning on line 146 in python/tank/commands/switch.py

View check run for this annotation

Codecov / codecov/patch

python/tank/commands/switch.py#L146

Added line #L146 was not covered by tests
raise TankError(
"Environment '%s' could not be loaded! Error reported: %s"
% (env_name, e)
Expand Down
2 changes: 1 addition & 1 deletion python/tank/descriptor/io_descriptor/appstore.py
Original file line number Diff line number Diff line change
Expand Up @@ -399,7 +399,7 @@
summary = sg_version_data.get("description")
url = sg_version_data.get("sg_detailed_release_notes").get("url")
except Exception:
pass
log.exception("Cant get changelog.")

Check warning on line 402 in python/tank/descriptor/io_descriptor/appstore.py

View check run for this annotation

Codecov / codecov/patch

python/tank/descriptor/io_descriptor/appstore.py#L402

Added line #L402 was not covered by tests
return (summary, url)

def _download_local(self, destination_path):
Expand Down
3 changes: 2 additions & 1 deletion python/tank/descriptor/io_descriptor/base.py
Original file line number Diff line number Diff line change
Expand Up @@ -260,7 +260,8 @@ def _find_latest_tag_by_pattern(self, version_numbers, pattern):
for version_num in version_numbers:
try:
version_split = list(map(int, version_num[1:].split(".")))
except Exception:
except Exception as e:
log.debug(e)
# this git tag is not on the expected form vX.Y.Z where X Y and Z are ints. skip.
continue

Expand Down
8 changes: 4 additions & 4 deletions python/tank/descriptor/io_descriptor/downloadable.py
Original file line number Diff line number Diff line change
Expand Up @@ -91,7 +91,7 @@
target_parent = os.path.dirname(target)
try:
filesystem.ensure_folder_exists(target_parent)
except Exception as e:
except OSError as e:

Check warning on line 94 in python/tank/descriptor/io_descriptor/downloadable.py

View check run for this annotation

Codecov / codecov/patch

python/tank/descriptor/io_descriptor/downloadable.py#L94

Added line #L94 was not covered by tests
if not os.path.exists(target_parent):
log.error("Failed to create directory %s: %s" % (target_parent, e))
raise TankDescriptorIOError(
Expand All @@ -104,7 +104,7 @@
# download completed without issue. Now create settings folder
metadata_folder = self._get_metadata_folder(temporary_path)
filesystem.ensure_folder_exists(metadata_folder)
except Exception as e:
except OSError as e:
# something went wrong during the download, remove the temporary files.
log.error(
"Failed to download into path %s: %s. Attempting to remove it."
Expand Down Expand Up @@ -141,7 +141,7 @@
% target
)

except Exception as e:
except OSError as e:

# if the target path already exists, this means someone else is either
# moving things right now or have moved it already, so we are ok.
Expand Down Expand Up @@ -188,7 +188,7 @@
filesystem.safe_delete_folder(temporary_path)
move_succeeded = True

except Exception as e:
except OSError as e:
# something during the copy went wrong. Attempt to roll back the target
# so we aren't left with any corrupt bundle cache items.
if os.path.exists(target):
Expand Down
7 changes: 4 additions & 3 deletions python/tank/descriptor/io_descriptor/git.py
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,8 @@
subprocess.STARTF_USESHOWWINDOW
subprocess.SW_HIDE
return True
except Exception:
except AttributeError as e:
log.debug("Terminal cant be hidden: %s" % e)

Check warning on line 38 in python/tank/descriptor/io_descriptor/git.py

View check run for this annotation

Codecov / codecov/patch

python/tank/descriptor/io_descriptor/git.py#L37-L38

Added lines #L37 - L38 were not covered by tests
return False


Expand Down Expand Up @@ -131,7 +132,7 @@
log.debug("Checking that git exists and can be executed...")
try:
output = _check_output(["git", "--version"])
except:
except SubprocessCalledProcessError:

Check warning on line 135 in python/tank/descriptor/io_descriptor/git.py

View check run for this annotation

Codecov / codecov/patch

python/tank/descriptor/io_descriptor/git.py#L135

Added line #L135 was not covered by tests
log.exception("Unexpected error:")
raise TankGitError(
"Cannot execute the 'git' command. Please make sure that git is "
Expand Down Expand Up @@ -291,7 +292,7 @@
# clone repo into temp folder
self._tmp_clone_then_execute_git_commands([], depth=1)
log.debug("...connection established")
except Exception as e:
except (OSError, SubprocessCalledProcessError) as e:

Check warning on line 295 in python/tank/descriptor/io_descriptor/git.py

View check run for this annotation

Codecov / codecov/patch

python/tank/descriptor/io_descriptor/git.py#L295

Added line #L295 was not covered by tests
log.debug("...could not establish connection: %s" % e)
can_connect = False
return can_connect
Expand Down
Loading
Loading