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

Fix handling of top-level exceptions #5560

Merged
merged 4 commits into from
Jul 28, 2022
Merged
Show file tree
Hide file tree
Changes from 3 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
7 changes: 7 additions & 0 deletions .changes/unreleased/Fixes-20220726-113636.yaml
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
kind: Fixes
body: Fix handling of top-level exceptions
time: 2022-07-26T11:36:36.824979-04:00
custom:
Author: gshank
Issue: "4357"
PR: "5560"
2 changes: 2 additions & 0 deletions core/dbt/events/base_types.py
Original file line number Diff line number Diff line change
Expand Up @@ -105,6 +105,8 @@ def level_tag(self) -> str:


# prevents an event from going to the file
# This should rarely be used in core code. It is currently
# only used in integration tests and for the 'clean' command.
class NoFile:
pass

Expand Down
2 changes: 1 addition & 1 deletion core/dbt/events/functions.py
Original file line number Diff line number Diff line change
Expand Up @@ -218,7 +218,7 @@ def create_log_line(e: T_Event, file_output=False) -> Optional[str]:
return create_info_text_log_line(e) # console output


# allows for resuse of this obnoxious if else tree.
# allows for reuse of this obnoxious if else tree.
# do not use for exceptions, it doesn't pass along exc_info, stack_info, or extra
def send_to_logger(l: Union[Logger, logbook.Logger], level_tag: str, log_line: str):
if not log_line:
Expand Down
3 changes: 2 additions & 1 deletion core/dbt/events/test_types.py
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
from dataclasses import dataclass
from .types import InfoLevel, DebugLevel, WarnLevel, ErrorLevel, ShowException, NoFile
from dbt.events.types import InfoLevel, DebugLevel, WarnLevel, ErrorLevel, ShowException
from dbt.events.base_types import NoFile


# Keeping log messages for testing separate since they are used for debugging.
Expand Down
12 changes: 7 additions & 5 deletions core/dbt/events/types.py
Original file line number Diff line number Diff line change
Expand Up @@ -94,15 +94,15 @@ class AdapterEventError(ErrorLevel, AdapterEventBase, ShowException):


@dataclass
class MainKeyboardInterrupt(InfoLevel, NoFile):
class MainKeyboardInterrupt(InfoLevel):
code: str = "Z001"

def message(self) -> str:
return "ctrl-c"


@dataclass
class MainEncounteredError(ErrorLevel, NoFile):
class MainEncounteredError(ErrorLevel):
e: BaseException
code: str = "Z002"

Expand All @@ -111,7 +111,7 @@ def message(self) -> str:


@dataclass
class MainStackTrace(DebugLevel, NoFile):
class MainStackTrace(ErrorLevel):
stack_trace: str
code: str = "Z003"

Expand Down Expand Up @@ -1353,6 +1353,8 @@ def message(self) -> str:
return "Error releasing connection for node {}: {!s}".format(self.node_name, self.exc)


# We don't write "clean" events to the log, because the clean command
# may have removed the log directory.
@dataclass
class CheckCleanPath(InfoLevel, NoFile):
path: str
Expand Down Expand Up @@ -2382,15 +2384,15 @@ def message(self) -> str:


@dataclass
class FlushEvents(DebugLevel, NoFile):
class FlushEvents(DebugLevel):
code: str = "Z042"

def message(self) -> str:
return "Flushing usage events"


@dataclass
class FlushEventsFailure(DebugLevel, NoFile):
class FlushEventsFailure(DebugLevel):
code: str = "Z043"

def message(self) -> str:
Expand Down
1 change: 0 additions & 1 deletion core/dbt/main.py
Original file line number Diff line number Diff line change
Expand Up @@ -142,7 +142,6 @@ def main(args=None):
exit_code = e.code

except BaseException as e:
traceback.print_exc()
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Given that this was an incidental inclusion in v1.2.0, I'd also be in favor of a PR that removed this line alone, and backporting that PR to 1.2.latest for inclusion in a v1.2.1 patch release

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm actually looking at two different problems, really, the lack of a stack trace for top level exceptions and catching exceptions in threads. I could make the current code the "lack of a stack trace" fix and backport it?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I like that idea!

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

good plan.

fire_event(MainEncounteredError(e=str(e)))
fire_event(MainStackTrace(stack_trace=traceback.format_exc()))
exit_code = ExitCodes.UnhandledError.value
Expand Down