[Bug] return in finally can swallow exceptions

[iritkatriel]

Is this a new bug in dbt-core?

• I believe this is a new bug in dbt-core
• I have searched the existing issues, and I could not find an existing issue for this bug

Current Behavior

in

dbt-core/core/dbt/task/run.py

Line 100 in f7b7935

return status, message

there is a return statement in a finally block, which would swallow any in-flight exception.

This means that if an exception other than DbtRuntimeError is raised from the try body, or any exception is raised from the except: clause, it will not propagate on as expected.

See also https://docs.python.org/3/tutorial/errors.html#defining-clean-up-actions.

Expected Behavior

Refactor so that there is no return in finally.

Steps To Reproduce

NA

Relevant log output

No response

Environment

- OS: Any
- Python: source code, main

Which database adapter are you using with dbt?

other (mention it in "Additional Context")

Additional Context

No response

[dbeatty10]

Thanks for reporting this @iritkatriel !

I'm curious how you discovered this. Did you actually notice something behaving incorrectly? Or is this some code you noticed that could theoretically have unintended behavior?

It looks to me like this code was added in #10699 which is not in dbt-core v1.8 but it slated for release in dbt-core v1.9.

[iritkatriel]

I'm curious how you discovered this.

I'm doing some analysis on return/break/continue in finally blocks (how often its use is correct/incorrect in real code). So running static analysis on lots of open source code. While I'm at it, I'm filing issues when I think it's worth looking into.

[dbeatty10]

Thanks again for raising this @iritkatriel 🤩

After discussing briefly with @peterallenwebb, he affirmed that we'd be interested in refactoring this section of code (specifically the part you mentioned):

dbt-core/core/dbt/task/run.py

Lines 88 to 100 in f7b7935

	def get_execution_status(sql: str, adapter: BaseAdapter) -> Tuple[RunStatus, str]:
	if not sql.strip():
	return RunStatus.Success, "OK"
	try:
	response, _ = adapter.execute(sql, auto_begin=False, fetch=False)
	status = RunStatus.Success
	message = response._message
	except DbtRuntimeError as exc:
	status = RunStatus.Error
	message = exc.msg
	finally:
	return status, message