Standardize init of exceptions

[ahopkins]

Needs some more cleanup. But this is an attempt to make raising exceptions a more consistent experience. It will be breaking however. There are a few existing exceptions that define their own __init__ with a signature that does not match. To the extent we can preserve those orders, I will. But, I think it will be best to convert the other arguments (status_code, extra, context, headers) to keyword.

[codecov[bot]]

Codecov Report

Base: 87.450% // Head: 87.393% // Decreases project coverage by -0.057% :warning:

Coverage data is based on head (7831fa1) compared to base (e499940). Patch coverage: 93.750% of modified lines in pull request are covered.

Additional details and impacted files

@@              Coverage Diff              @@
##              main     #2545       +/-   ##
=============================================
- Coverage   87.450%   87.393%   -0.058%     
=============================================
  Files           69        69               
  Lines         5602      5608        +6     
  Branches       974       974               
=============================================
+ Hits          4899      4901        +2     
- Misses         509       513        +4     
  Partials       194       194               

Impacted Files	Coverage Δ
sanic/exceptions.py	94.791% <93.750%> (-4.098%)	:arrow_down:

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

:umbrella: View full report at Codecov.
:loudspeaker: Do you have feedback about the report comment? Let us know in this issue.

[ahopkins]

I am a bit torn.

On the one hand: we have exceptions that are a bit of a mess. There is not a consistent __init__, and properties are not guaranteed to be there.

On the other: exceptions are very fundamental. Changing them will break for users.

---

I thought about ways to solve this:

1. monkeypatch until we can deprecate
2. providing a legacy and a new module

Neither of these solutions will work, and have their own problems. Anyone else have some thoughts?

Right now I am leaning towards fixing what we can, and leaving the breaking changes (a uniform constructor) for another time.

[Tronic]

I generally stand with "break it now to fix all of the future" (as opposed to keeping broken APIs for compatibility). But yes, at a bare minimum one should be able to write code that then works both with old and new Sanic versions, even if it requires changes to some old code.

If it is about kwargs' names being changed, I suppose both names could be supported on init functions as a deprecation path. Removal of positional args is harder but at least one can then always fix the code to use kwargs instead and it will work with older Sanic too. Not a big fan of either one of the two solutions that you suggest, as both would increase complexity and confusion quite a bit.

[ahopkins]

If it is about kwargs' names being changed, I suppose both names could be supported on init functions as a deprecation path. Removal of positional args is harder but at least one can then always fix the code to use kwargs instead and it will work with older Sanic too. Not a big fan of either one of the two solutions that you suggest, as both would increase complexity and confusion quite a bit.

Yup. Which is why I ruled them out.

The specific issue is this:

class SanicException(Exception):
    def __init__(
        self,
        message: Optional[Union[str, bytes]] = None,
        status_code: Optional[int] = None,
        quiet: Optional[bool] = None,
        context: Optional[Dict[str, Any]] = None,
        extra: Optional[Dict[str, Any]] = None,
    ) -> None:
        ...
class MethodNotAllowed(SanicException):
    def __init__(self, message, method, allowed_methods):
        ...

class Unauthorized(SanicException):
    def __init__(self, message, status_code=None, scheme=None, **kwargs):
        ...

We have subclasses with incompatible signatures. I would really like to get to to keyword args (except message), and allow the subtypes to add their own.

class SanicException(Exception):
    def __init__(
        self,
        message: Optional[Union[str, bytes]] = None,
        *,
        status_code: Optional[int] = None,
        quiet: Optional[bool] = None,
        context: Optional[Dict[str, Any]] = None,
        extra: Optional[Dict[str, Any]] = None,
    ) -> None:
        ...
class MethodNotAllowed(SanicException):
    def __init__(
        self,
        message,
        method,
        allowed_methods
        *,
        status_code: Optional[int] = None,
        quiet: Optional[bool] = None,
        context: Optional[Dict[str, Any]] = None,
        extra: Optional[Dict[str, Any]] = None,
    ):
        ...

class Unauthorized(SanicException):
    def __init__(
        self,
        message,
        status_code=None,
        scheme=None
        *,
        status_code: Optional[int] = None,
        quiet: Optional[bool] = None,
        context: Optional[Dict[str, Any]] = None,
        extra: Optional[Dict[str, Any]] = None,
    ):
        ...

So, yes, the issue is we have bad subclassing that messes up positional args.