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

remove the exception handler / improve logging out of the box (esp. when settings.DEBUG=True) #275

Open
olivierdalang opened this issue Apr 16, 2024 · 2 comments
Labels
work-in-progress Stale action

Comments

@olivierdalang
Copy link

olivierdalang commented Apr 16, 2024

Hi !

Out of the box, the package is very silent whenever issues happen.

It's not trivial to set logging correctly (it's not with native Django, and it's even less with possible not clearly defined interference between the native django settings.LOGGING and settings.DEBUG and their counterparts in settings.SAML2_AUTH).

In my case, after trying for over one hour to get output, I got rid of the exception handler and then only got a nice and clear trace indicating the exact issue (which was Cannot find ['xmlsec1']). From the issue tracker, it seem many people struggle in a similar way with uninformative error messages (Sorry, you are not allowed to access this app).

What about completely removing the exception handler when Django's settings.DEBUG=True, so that we get regular debug information ? Arguably even so when DEBUG=False, as I don't see a good reason for this package to provide some custom exception handling ?

Thanks !

For reference, here's a dirty monkey-patch to disable the exception handler (in your settings.py):

# monkey-patch out django_saml2_auth's exception handler to get proper traces
# https://github.com/grafana/django-saml2-auth/issues/275
if DEBUG:
    import django_saml2_auth.utils
    django_saml2_auth.utils.exception_handler = lambda func: func
@mostafa
Copy link
Member

mostafa commented Apr 25, 2024

Hey @olivierdalang,

I think the root cause is correctly identified by @73VW in #277 and I fixed it in #281. I hope this help resolve the issue after I release a new version, which will happen very soon. In the meantime use the main branch to test the changes. I'd be happy to see this resolved.

Copy link

This issue is stale because it has been open 30 days with no activity. Remove stale label or comment or this will be closed in 5 days.

@github-actions github-actions bot added the no-issue-activity Stale action label May 26, 2024
@mostafa mostafa added work-in-progress Stale action and removed no-issue-activity Stale action labels May 27, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
work-in-progress Stale action
Projects
None yet
Development

No branches or pull requests

2 participants