Skip to content
This repository has been archived by the owner on Dec 4, 2024. It is now read-only.

Improve error messages; fix errant 500 error #97

Closed
wants to merge 1 commit into from

Conversation

drewbanin
Copy link
Contributor

Note: going to see if we can make this kind of change in Core, as i think that's a better home for this kind of logic...

Two changes here:

  1. Improving (some) error messages raised by Core
  2. Convert 500 --> 400 by differentiating between None and and empty string when returning compiled SQL

Error message diff

Big idea here: the resource_type, unique_id, path, and name of SqlOperation nodes are often included in exception messages in Core. SqlOperation properties are pretty jank/confusing, and tend to look like:

Most exceptions in dbt follow the format:

Compilation Error in [resource_type] [name]
[Error]

Errors from dbt Core that pertain to Sql Operations (eg. compile queries) tend to look like:

Compilation Error in sql operation name (from remote system.sql)
unexpected '}'   line 1   {{ 1 + 1}

I think that sql operation name (from remote system.sql) is possibly the most confusing fragment of an error message that I've ever seen, so trying to change that here 😓 . Note that this PR also removes logic that replaces newlines with spaces. I think having separate lines for these error messages makes them a lot more intelligible (especially if there is a stack trace for macro calls), but I'm open to changing that back if anyone has alternative thoughts or opinions. Practical examples below!

Syntax errors

before

Compilation Error in sql operation name (from remote system.sql) unexpected '}'   line 1   {{ 1 + 1}

After

dbt Error: unexpected '}'
  line 1
    {{ 1 + 1}

Ref not found errors

These are better than before, but still not ideal. dbt is trying to tell us that our sql operation node (named Sql Operation 'sql operation.demo_data.query' (from remote system.sql)) depends on a node called badmodel

Before

Compilation Error in sql operation query (from remote system.sql) Sql Operation 'sql operation.demo_data.query' (from remote system.sql) depends on a node named 'badmodel' which was not found

After

dbt Error: Sql Operation 'sql operation.demo_data.name' (from remote system.sql) depends on a node named 'badmodel' which was not found

Macro stack traces

Before
TODO

After
TODO

@cla-bot cla-bot bot added the cla:yes label Sep 23, 2022
@drewbanin
Copy link
Contributor Author

Closing this because i convinced myself that the right home for this change is in Core.

@jtcohen6 - would love to get your preliminary thoughts on something like this: https://github.com/dbt-labs/dbt-core/compare/feature/better-error-messages-for-sql-operations?expand=1

@drewbanin drewbanin closed this Sep 23, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant