Fix empty str(exception) when initialized with kwargs

[BoWuGit]

Add a super().__init__ call to sub class of Python standard Exception, for two reasons:

• It's convention to call it for sub class
• Like the title says, when HTTPException is initialized with kwargs, str(HTTPException) is empty, which is just the case I met in this pytest error case.

Thanks for your attention.

[zanieb]

This makes sense to me after reading the upstream issue, thanks for contributing!

It might be nice to just add a new test case for this instead of changing the existing repr test case.

[zanieb]

Suggested change

	assert repr(exception) == (f"HTTPException(status_code=404, detail='{detail}')")
	assert repr(exception) == f"HTTPException(status_code=404, detail='{detail}')"

[zanieb]

Suggested change

	assert repr(exception) == (f"WebSocketException(code=1008, reason='{reason}')")
	assert repr(exception) == f"WebSocketException(code=1008, reason='{reason}')"

[Kludex]

Please implement the __str__ instead of super().__init__(self.detail).

Also, on the __str__, I think we can have the same implementation as the __repr__.

On DRF, they use only the detail: https://github.com/encode/django-rest-framework/blob/376a5cbbba3f8df9c9db8c03a7c8fa2a6e6c05f4/rest_framework/exceptions.py#L99C1-L117

On flask, they use all the information: https://github.com/pallets/werkzeug/blob/82b5ac433c54b8d03cb0de9fdc16a4c58b935860/src/werkzeug/exceptions.py#L65-L85

[zanieb]

@Kludex for what it's worth, I don't think that __str__ and __repr__ implementations should match if you want standard behavior

❯ python -c "print(str(ValueError('test')))"
test
❯ python -c "print(repr(ValueError('test')))"
ValueError('test')

Using the __repr__ implementation would break expectations in downstream libraries that format exceptions e.g. https://github.com/PrefectHQ/prefect/blob/92e3e6de417bf542c9fca201fd396dba7301665c/src/prefect/states.py#L121 would display the exception type twice

The linked Flask code omits the type from the __str__ https://github.com/pallets/werkzeug/blob/82b5ac433c54b8d03cb0de9fdc16a4c58b935860/src/werkzeug/exceptions.py#L164-L170

[Kludex]

The linked Flask code omits the type from the str pallets/werkzeug@82b5ac4/src/werkzeug/exceptions.py#L164-L170

Yeah, I pasted the wrong part of the implementation. By "they use all the information" I mean that the __str__ uses all the arguments from the constructor.

Using the repr implementation would break expectations in downstream libraries that format exceptions e.g. PrefectHQ/prefect@92e3e6d/src/prefect/states.py#L121 would display the exception type twice

Fair enough.

Is there any literature that has a recommendation on how the __str__ should look like, or should we just copy flask? 🤔

[BoWuGit]

@Kludex Is there any literature that has a recommendation on how the str should look like, or should we just copy flask? 🤔

For sub class of Python Exception, str(Exception) defaults to str(Exception.args) if you don't call super().__init__ and don't override __str__ method, for example:

>>> str(HTTPException(404, "Not found"))
"(404, 'Not found')"

On DRF, they use only the detail: encode/django-rest-framework@376a5cb/rest_framework/exceptions.py#L99C1-L117

Yes, str(Exception) only displays the detail, and they also have a get_full_details method, so for simplicity consideration, I think we can refer Flask's implemention.

[BoWuGit]

I have re-implemented it referring Flask, https://github.com/pallets/werkzeug/blob/main/src/werkzeug/exceptions.py#L164. And I think it is somehow backward compatible in this way, because str(Exception) defaults to str(Exception.args) if you don't call super().__init__ and don't override __str__ method, so both code and detail should be outputted.

[Kludex]

Why do we need super().__init__()?

[BoWuGit]

I think it's desired practice in Python to always call super().__init__, and this pytest error case I met with is related with it more or less. Giving two links to explain:

Is it necessary to call super().init() explicitly in python?

If you override the init method of the superclass, then the init method of the subclass needs to explicitly call it if that is the intended behavior, yes.

Is calling the superclass constructor in a subclass really important?

If you add variables to self in the constructor of Class and don't call Class.init() in the constructor of Subclass, then these variables will not be in your Subclass object.

[Kludex]

this pytest-dev/pytest#11073 I met with is related with it more or less

Doesn't the __str__ already solves that?

[Kludex]

Thanks @BoWuGit 👍

[BoWuGit]

Happy to be merged, thanks again.