Skip to content

Commit

Permalink
Mark BuildEnvironmentError exceptions as Warning and do not log them
Browse files Browse the repository at this point in the history
There are some exceptions risen while building documentation that are
considered an ERROR from the Build perspective, but for our
application are just a WARNING and shouldn't be logged as ERROR (sent
to Sentry).

It's not an ERROR in our application but a WARNING that the user's
documentation couldn't be built.
  • Loading branch information
humitos committed Aug 8, 2018
1 parent 3593dc9 commit 1dd850a
Show file tree
Hide file tree
Showing 3 changed files with 76 additions and 34 deletions.
57 changes: 39 additions & 18 deletions readthedocs/doc_builder/environments.py
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@
from requests.exceptions import ConnectionError

from .exceptions import (BuildEnvironmentException, BuildEnvironmentError,
BuildEnvironmentWarning, BuildEnvironmentCreationFailed)
BuildEnvironmentWarning, BuildEnvironmentCreationFailed, VersionLockedError, ProjectBuildsSkippedError, YAMLParseError, BuildTimeoutError)
from .constants import (DOCKER_SOCKET, DOCKER_VERSION, DOCKER_IMAGE,
DOCKER_LIMITS, DOCKER_TIMEOUT_EXIT_CODE,
DOCKER_OOM_EXIT_CODE, SPHINX_TEMPLATE_DIR,
Expand All @@ -40,9 +40,11 @@

__all__ = (
'api_v2',
'BuildCommand', 'DockerBuildCommand',
'BuildCommand',
'DockerBuildCommand',
'LocalEnvironment',
'LocalBuildEnvironment', 'DockerBuildEnvironment',
'LocalBuildEnvironment',
'DockerBuildEnvironment',
)


Expand Down Expand Up @@ -409,6 +411,16 @@ class BuildEnvironment(BaseEnvironment):
successful
"""

# Exceptions considered ERROR from a Build perspective but as a WARNING for
# the application itself. These exception are logged as warning and not sent
# to Sentry.
WARNING_EXCEPTIONS = (
VersionLockedError,
ProjectBuildsSkippedError,
YAMLParseError,
BuildTimeoutError,
)

def __init__(self, project=None, version=None, build=None, config=None,
record=True, environment=None, update_on_success=True):
super(BuildEnvironment, self).__init__(project, environment)
Expand Down Expand Up @@ -445,21 +457,30 @@ def handle_exception(self, exc_type, exc_value, _):
a failure and the context will be gracefully exited.
"""
if exc_type is not None:
if not issubclass(exc_type, BuildEnvironmentWarning):
log.error(LOG_TEMPLATE
.format(project=self.project.slug,
version=self.version.slug,
msg=exc_value),
exc_info=True,
extra={
'stack': True,
'tags': {
'build': self.build.get('id'),
'project': self.project.slug,
'version': self.version.slug,
},
})
self.failure = exc_value
log_level_function = None
if issubclass(exc_type, BuildEnvironmentWarning):
log_level_function = log.warning
elif exc_type in self.WARNING_EXCEPTIONS:
log_level_function = log.warning
else:
log_level_function = log.error

log_level_function(
LOG_TEMPLATE.format(
project=self.project.slug,
version=self.version.slug,
msg=exc_value,
),
exc_info=True,
extra={
'stack': True,
'tags': {
'build': self.build.get('id'),
'project': self.project.slug,
'version': self.version.slug,
},
})
self.failure = exc_value
return True

def record_command(self, command):
Expand Down
28 changes: 28 additions & 0 deletions readthedocs/doc_builder/exceptions.py
Original file line number Diff line number Diff line change
Expand Up @@ -31,5 +31,33 @@ class BuildEnvironmentCreationFailed(BuildEnvironmentError):
)


class VersionLockedError(BuildEnvironmentError):

message = ugettext_noop(
'Version locked, retrying in 5 minutes.',
)


class ProjectBuildsSkippedError(BuildEnvironmentError):

message = ugettext_noop(
'Builds for this project are temporarily disabled',
)


class YAMLParseError(BuildEnvironmentError):

GENERIC_WITH_PARSE_EXCEPTION = ugettext_noop(
'Problem parsing YAML configuration. {exception}',
)


class BuildTimeoutError(BuildEnvironmentError):

message = ugettext_noop(
'Build exited due to time out',
)


class BuildEnvironmentWarning(BuildEnvironmentException):
pass
25 changes: 9 additions & 16 deletions readthedocs/projects/tasks.py
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,7 @@
from readthedocs.doc_builder.constants import DOCKER_LIMITS
from readthedocs.doc_builder.environments import (LocalBuildEnvironment,
DockerBuildEnvironment)
from readthedocs.doc_builder.exceptions import BuildEnvironmentError
from readthedocs.doc_builder.exceptions import BuildEnvironmentError, VersionLockedError, ProjectBuildsSkippedError, YAMLParseError, BuildTimeoutError
from readthedocs.doc_builder.loader import get_builder_class
from readthedocs.doc_builder.python_environments import Virtualenv, Conda
from readthedocs.projects.models import APIProject
Expand Down Expand Up @@ -406,23 +406,19 @@ def run_setup(self, record=True):
# Environment used for code checkout & initial configuration reading
with self.setup_env:
if self.project.skip:
raise BuildEnvironmentError(
_('Builds for this project are temporarily disabled'))
raise ProjectBuildsSkippedError
try:
self.setup_vcs()
except vcs_support_utils.LockTimeout as e:
self.task.retry(exc=e, throw=False)
raise BuildEnvironmentError(
'Version locked, retrying in 5 minutes.',
status_code=423
)

raise VersionLockedError
try:
self.config = load_yaml_config(version=self.version)
except ConfigError as e:
raise BuildEnvironmentError(
'Problem parsing YAML configuration. {0}'.format(str(e))
)
raise YAMLParseError(
YAMLParseError.GENERIC_WITH_PARSE_EXCEPTION.format(
exception=str(e),
))

if self.setup_env.failure or self.config is None:
self._log('Failing build because of setup failure: %s' % self.setup_env.failure)
Expand Down Expand Up @@ -482,12 +478,9 @@ def run_build(self, docker, record):
build_id = self.build.get('id')
except vcs_support_utils.LockTimeout as e:
self.task.retry(exc=e, throw=False)
raise BuildEnvironmentError(
'Version locked, retrying in 5 minutes.',
status_code=423
)
raise VersionLockedError
except SoftTimeLimitExceeded:
raise BuildEnvironmentError(_('Build exited due to time out'))
raise BuildTimeoutError

# Finalize build and update web servers
if build_id:
Expand Down

0 comments on commit 1dd850a

Please sign in to comment.