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

[Feature] When Jinja fails to render a model due TypeError should raise an exception with the node information #10856

Open
3 tasks done
devmessias opened this issue Oct 15, 2024 · 1 comment
Labels
awaiting_response enhancement New feature or request

Comments

@devmessias
Copy link
Contributor

devmessias commented Oct 15, 2024

Is this your first time submitting a feature request?

  • I have read the expectations for open source contributors
  • I have searched the existing issues, and I could not find an existing issue for this feature
  • I am requesting a straightforward extension of existing dbt functionality, rather than a Big Idea better suited to a discussion

Describe the feature

A common headache that I face with DBT is when a TypeError occurs during the Jinja rendering phase. Currently, when such errors arise, especially due to None values, it's challenging to identify where the problem lies. This is an old issue, and others have already complained about it, making it difficult to find the root cause in other problems see: dbt-labs-experimental-features/issues/37, #4596, #8276, #9033, etc.
also, this comment
#4596 (comment)

Maybe TypeError should be kind of RuntimeError like this

Runtime Error in model ex2 (models/ex2.sql)
  TypeError:'NoneType' object is not iterable
 TypeError: Error in model ex2 (models/ex2.sql)
  NoneType' object is not iterable

Example

-- models/my_model.sql
{% set mylist = None %}

{% for item in mylist %}

  select '{{ item }}' as my_item
  {{ 'union all' if not loop.last }}

{% endfor %}

Describe alternatives you've considered

  • Create a DbtRuntimeTypeError
  • Use that inside the catch_jinja context manager
@contextmanager
def catch_jinja(node: Optional[_NodeProtocol] = None) -> Iterator[None]:
    try:
        yield
    except jinja2.exceptions.TemplateSyntaxError as e:
        e.translated = False
        raise CompilationError(str(e), node) from e
    except jinja2.exceptions.UndefinedError as e:
        raise UndefinedMacroError(str(e), node) from e
    except jinja2.exceptions.TemplateRuntimeError as e:
        raise CompilationError(str(e), node) from e
    except RenderTypeError as e:
        raise DbtRuntimeTypeError(str(e), node) from e
    except CompilationError as exc:
        exc.add_node(node)
        raise

The following approach is really bad. I believe we should just check for TypeError, raising a general DbtRuntimeError can mask the root cause of unexpected error

@contextmanager
def catch_jinja(node: Optional[_NodeProtocol] = None) -> Iterator[None]:
    try:
        yield
    except jinja2.exceptions.TemplateSyntaxError as e:
        e.translated = False
        raise CompilationError(str(e), node) from e
    except jinja2.exceptions.UndefinedError as e:
        raise UndefinedMacroError(str(e), node) from e
    except jinja2.exceptions.TemplateRuntimeError as e:
        raise CompilationError(str(e), node) from e
    except CompilationError as exc:
        exc.add_node(node)
        raise
    except Exception as e:
        class_error = e.__class__.__name__
        raise DbtRuntimeError(f"{class_error}:{e}", node) from e

Who will this benefit?

everyone who spend minutes or hours trying to fix an issue due the billion dolar mistake

https://www.infoq.com/presentations/Null-References-The-Billion-Dollar-Mistake-Tony-Hoare/

Are you interested in contributing this feature?

yes

Anything else?

No response

@devmessias devmessias added enhancement New feature or request triage labels Oct 15, 2024
@devmessias devmessias changed the title [Feature] When Jinja fails to render a model due TypeError should display the model and line number. [Feature] When Jinja fails to render a model due TypeError should raise an exception with the node information Oct 15, 2024
@dbeatty10
Copy link
Contributor

Love this idea @devmessias 🤩

It seems like we can close this as a duplicate of dbt-labs/dbt-common#206?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
awaiting_response enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

2 participants