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

raise Err() from e and e.add_note() lose information in validation errors #6498

Closed
3 of 13 tasks
zakstucke opened this issue Jul 7, 2023 · 10 comments · Fixed by #7626
Closed
3 of 13 tasks

raise Err() from e and e.add_note() lose information in validation errors #6498

zakstucke opened this issue Jul 7, 2023 · 10 comments · Fixed by #7626
Assignees
Labels
feature request help wanted Pull Request welcome

Comments

@zakstucke
Copy link
Contributor

zakstucke commented Jul 7, 2023

Initial Checks

  • I have searched Google & GitHub for similar requests and couldn't find anything
  • I have read and followed the docs and still think this feature is missing

Description

Edit: 2 is actually a regression from v1, in 1.10.11 the outputted error was Must be greater than 5 (type=assertion_error; __notes__=['ERR NOTE!']).

Errors in V2 are on another level to V1, but error manipulation still loses information in the final ValidationError outputted by pydantic when:

  1. Catching an error, raising an outer error using the from e syntax
    • with pd: the inner error is lost
  2. adding notes to errors using e.add_note()
    • with pd: the note is lost

In both cases information is lost, which can be very useful when complex multi-func validation is going on to add context to inner errors at the top level.

I don't think either are a security concern being enabled by default in pydantic, both are opt in with from e and add_note respectively.

from pydantic import BaseModel, field_validator

class Foo(BaseModel):
    foo: int
    bar: int

    @field_validator("foo")
    def val_foo(cls, v: int) -> int:
        try:
            assert v > 5, "Must be greater than 5"
        except AssertionError as e:
            raise ValueError("OUTER EXC!") from e

        return v

    @field_validator("bar")
    def val_bar(cls, v: int) -> int:
        try:
            assert v > 5, "Must be greater than 5"
        except AssertionError as e:
            e.add_note("ERR NOTE!")
            raise e

        return v
        

Foo(foo=4, bar=4)

Results in

Traceback (most recent call last):
  File "mypy_playground.py", line 28, in <module>
    Foo(foo=4, bar=4)
  File "venv/lib/python3.11/site-packages/pydantic/main.py", line 150, in __init__
    __pydantic_self__.__pydantic_validator__.validate_python(data, self_instance=__pydantic_self__)
pydantic_core._pydantic_core.ValidationError: 2 validation errors for Foo
foo
  Value error, OUTER EXC! [type=value_error, input_value=4, input_type=int]
    For further information visit https://errors.pydantic.dev/2.1.2/v/value_error
bar
  Assertion failed, Must be greater than 5 [type=assertion_error, input_value=4, input_type=int]
    For further information visit https://errors.pydantic.dev/2.1.2/v/assertion_error

Native errors when outside pydantic:

Traceback (most recent call last):
  File "mypy_playground.py", line 7, in <module>
    raise e
  File "mypy_playground.py", line 4, in <module>
    assert v > 5, "Must be greater than 5"
           ^^^^^
AssertionError: Must be greater than 5
ERR NOTE!
Traceback (most recent call last):
  File "mypy_playground.py", line 4, in <module>
    assert v > 5, "Must be greater than 5"
           ^^^^^
AssertionError: Must be greater than 5

The above exception was the direct cause of the following exception:

Traceback (most recent call last):
  File "mypy_playground.py", line 6, in <module>
    raise ValueError("OUTER EXC!") from e
ValueError: OUTER EXC!
             pydantic version: 2.0.2
        pydantic-core version: 2.1.2 release build profile
                 install path: venv/lib/python3.11/site-packages/pydantic
               python version: 3.11.4 (main, Jun  7 2023, 12:45:48) [GCC 11.3.0]
                     platform: Linux-5.19.0-46-generic-x86_64-with-glibc2.35
     optional deps. installed: ['devtools', 'email-validator', 'typing-extensions']

Affected Components

Selected Assignee: @lig

@pydantic-hooky pydantic-hooky bot added the unconfirmed Bug not yet confirmed as valid/applicable label Jul 7, 2023
@lig
Copy link
Contributor

lig commented Jul 7, 2023

This comment is based on Pydantic V1 behavior

With Pydantic it's slightly different by design. Validation errors in Pydantic aren't fatal, the process doesn't crash. Errors are being collected and are ready to be presented to a user. Important, the information is not lost.

In the from error case the original error could contain some private data and a validator doesn't have enough knowledge to protect the data. So, it should be a user responsibility to ensure what data exactly should go into a string representation of a ValidationError. Something like that should work (but this should not be treated as an universal recipe because of reasons mentioned above):

        try:
            assert v > 5, "Must be greater than 5"
        except AssertionError as e:
            raise ValueError(f"OUTER EXC!: {e}")

In the add_note case it could be improved indeed. Currently, Pydantic creates it's own ValidationError and wraps any error thrown from a validator with ErrorWrapper. ErrorWrapper gives no effort to disply BaseException.__notes__ as defined in PEP 678. However, it's unclear to me at the moment how these notes should be represented.

Please, note that all of the above relates to a string representation of the ValidationError. At the moment all the desired information is still accessible programmatically like this:

try:
    Foo(foo=4, bar=4)
except ValidationError as e:
    for inner_error in e.raw_errors:
        print(f'{inner_error} cause: {inner_error.exc.__cause__}')
        print(f'{inner_error} notes: {inner_error.exc.__notes__}')

Update: the comment was misleading

@lig lig removed the unconfirmed Bug not yet confirmed as valid/applicable label Jul 7, 2023
@lig lig changed the title raise Err() from e and e.add_note() lose information in validation errors Enhance ValidationError string representation with __cause__ and __notes__ data Jul 7, 2023
@zakstucke
Copy link
Contributor Author

zakstucke commented Jul 7, 2023

Apologies, accidentally closed, reopened.

I'd disagree on a few points here:

  • Your workaround with raw_errors was valid in V1. But in V2 raw_errors seems to no longer exists, I think the rust-side throws it all away. The lowest-level public api I can see is errors() which returns list[ErrorDetails] see below, in both cases the extra information referenced seems to not be available programatically:
try:
    Foo(foo=4, bar=4)
except ValidationError as e:
    for inner_error in e.errors():
        pprint.pprint(inner_error, indent=2)
{ 'ctx': {'error': 'OUTER EXC!'},
  'input': 4,
  'loc': ('foo',),
  'msg': 'Value error, OUTER EXC!',
  'type': 'value_error',
  'url': 'https://errors.pydantic.dev/2.1.2/v/value_error'}
{ 'ctx': {'error': 'Must be greater than 5'},
  'input': 4,
  'loc': ('bar',),
  'msg': 'Assertion failed, Must be greater than 5',
  'type': 'assertion_error',
  'url': 'https://errors.pydantic.dev/2.1.2/v/assertion_error'}
  • In terms of process crashing, in a lot of data processing use cases outside FastApi, including mine, validation errors do crash the process, and I spend a lot of time reading through error traces coming from pydantic. It would be great to use the standard python syntax when desired for non client facing validation issues without manually producing stringified stack traces using traceback.
  • The inner error could definitely contain private information for sure. However, the from e syntax is entirely optional, when a dev specifies from e they are specifically requesting access to the inner trace as well as the outer to prevent it being discarded, so I would argue there's no security concern from pydantic's point of view.

@zakstucke zakstucke reopened this Jul 7, 2023
@pydantic-hooky pydantic-hooky bot added the unconfirmed Bug not yet confirmed as valid/applicable label Jul 7, 2023
@lig
Copy link
Contributor

lig commented Jul 7, 2023

@zakstucke it could be that I was looking into pydantic.v1 package in my previous comment, sorry 🤦🏻‍♂️.

Let me reproduce the issues you've reported and get back to you.

@lig lig changed the title Enhance ValidationError string representation with __cause__ and __notes__ data raise Err() from e and e.add_note() lose information in validation errors Jul 7, 2023
@lig
Copy link
Contributor

lig commented Jul 7, 2023

@zakstucke You're right 😇. In Pydantic V2 in {'ctx': {'error': 'error text'}} it stores a string representation of the error only and looses a lot of information.

Ideally, I would like to have an actual error object stored there.

@lig
Copy link
Contributor

lig commented Jul 7, 2023

@lig lig added help wanted Pull Request welcome and removed unconfirmed Bug not yet confirmed as valid/applicable labels Jul 7, 2023
@lig
Copy link
Contributor

lig commented Jul 7, 2023

@zakstucke Thanks you for the request. I cannot say when we will be able to implement this in Pydantic V2. This will definitely involve improving error handling in pydantic-core. A PR implementing this would be welcome.

@zakstucke
Copy link
Contributor Author

No worries, thanks!

@zakstucke
Copy link
Contributor Author

zakstucke commented Jul 15, 2023

@adriangb now that pydantic/pydantic-core#753 has been merged in, the exception info can now be accessed externally:

from pydantic import BaseModel, field_validator, ValidationError

def check(v: int):
    if v < 0:
        raise ValueError("INNER ERR")
    return v

class Foo(BaseModel):
    foo: int

    @field_validator("foo")
    def check_foo(cls, v):
        if v < 0:
            try:
                check(v)
            except ValueError as e:
                new_err = ValueError("OUTER ERR")
                new_err.add_note("NOTE")
                raise new_err from e
        return v

try:
    Foo(foo=-1)
except ValidationError as e:
    for error in e.errors():
        print(error["ctx"]["error"])
        print(error["ctx"]["error"].__notes__)
        print(error["ctx"]["error"].__cause__)

    raise e

Which prints:

OUTER ERR
['NOTE']
INNER ERR

Awesome thanks for that!

However the traceback still formats as:

ValidationError: 1 validation error for Foo
foo
  Value error, OUTER ERR [type=value_error, input_value=-1, input_type=int]
    For further information visit https://errors.pydantic.dev/2.0.3/v/value_error

This was one of the most annoying things about V1, as it made debugging layers of errors so much harder than it usually is in native Python.

Is there a specific reason to not allow showing these information chains (specifically enabled by the dev with from e and add_note() aka no security concern), or could these be implemented?

@adriangb
Copy link
Member

I suppose it could be implemented

@zakstucke
Copy link
Contributor Author

@adriangb check out pydantic/pydantic-core#780 for a draft that would solve this

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature request help wanted Pull Request welcome
Projects
None yet
4 participants