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

Log exception in Webhook.process? #370

Open
blueyed opened this issue Sep 21, 2017 · 5 comments
Open

Log exception in Webhook.process? #370

blueyed opened this issue Sep 21, 2017 · 5 comments

Comments

@blueyed
Copy link
Contributor

blueyed commented Sep 21, 2017

Do you think it makes sense to log exceptions in https://github.com/pinax/pinax-stripe/blob/34e6a3cec7a7df2d60c5b2696066c8dba8947e75/pinax/stripe/webhooks.py#L95-L96?

It will create an EventProcessingException object already (via https://github.com/pinax/pinax-stripe/blob/34e6a3cec7a7df2d60c5b2696066c8dba8947e75/pinax/stripe/actions/exceptions.py#L7-L23), but I think it would be good to use logger.exception here, for more visibility (server logs etc).

Alternatively we could use the post-save signal on EventProcessingException to do it manually.

@blueyed
Copy link
Contributor Author

blueyed commented Sep 21, 2017

It makes a lot of sense, since you might be unable to create a EventProcessingException in case of a IntegrityError ('duplicate key value violates unique constraint'), where trying to create it could raise TransactionManagementError("An error occurred in the current transaction. You can't execute queries until the end of the 'atomic' block.",).

@paltman paltman added this to the Samwise milestone Oct 20, 2017
@paltman
Copy link
Contributor

paltman commented Oct 20, 2017

Are you answering your own question and therefore we can close this?

@blueyed
Copy link
Contributor Author

blueyed commented Oct 20, 2017

No, I am rather giving a reason for changing this in the comment.

@paltman
Copy link
Contributor

paltman commented Oct 20, 2017

So you are saying there are some exceptions that we'd be prevented from storing in the database?

I'd like to store as much in the database as possible, and only when all else fails, use the logger.

@paltman
Copy link
Contributor

paltman commented Oct 30, 2017

I think we should still try to log the exception in the database. If you want to also add a logger line for server logs prior to trying to log into the database I'm fine with that

@paltman paltman modified the milestones: Samwise, Rosie Oct 30, 2017
@paltman paltman modified the milestones: Rosie, December 2017 Sprint Dec 1, 2017
@paltman paltman removed this from the December 2017 Sprint milestone Nov 25, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants