-
-
Notifications
You must be signed in to change notification settings - Fork 312
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
Impossible to catch exceptions that aren't OneLogin_Saml2_Error #40
Comments
Right now I think that OneLogin_Saml2_Error is used to catch errors related to the SAML settings or when SAML messages not detected (bad SSO/SLO flow). Im agree that we may improve the way we take care of exceptions and help to the dev on the debug process. |
Would you be willing to accept a PR with changes to use a more specific exception class? |
Something like OneLogin_Saml2_ValidationError ? |
After looking into errors.py, it seems the Thoughts on whether I should add a new error code https://github.com/onelogin/python3-saml/blob/master/src/onelogin/saml2/errors.py#L32 |
You can see that those error codes are related to settings or anormal SAML flow. I think we can create a similar OneLogin_Saml2_ValidationError with codes that describes what validation failed, then we get a more complete error handle. What do you think? |
Sounds good to me! |
@nickw444 Do you have more custom ValidationException to submit? If not I will take care of this commit and implement it. |
I'm taking care of it. |
Thanks |
I'm having an issue where the library is throwing an
Exception
that is of the baseException
type. I want to handle errors gracefully that occur within this library, but don't really want to catch every single exception that occurs.Is there a particular reason this library uses
OneLogin_Saml2_Error
in some places, but others, like in https://github.com/onelogin/python3-saml/blob/master/src/onelogin/saml2/response.py#L301, just useException
? Would it be possible to introduce a new class ofException
which is used in place of these so that they can be caught more easily without catching allException
types.Cheers
The text was updated successfully, but these errors were encountered: