Skip to content

Conversation

@dcramer
Copy link
Member

@dcramer dcramer commented Dec 9, 2016

Refs GH-40

@dcramer
Copy link
Member Author

dcramer commented Dec 9, 2016

I'm not sure this is the best approach to logging these errors and its not something we yet do anywhere else in PHP.

@stayallive @mitsuhiko either of you have any thoughts? I'd basically want to take this, shift it into the RavenClient class (e.g. RavenClient::logInternalError) and standardize wherever else we might be throwing away failures.

@stayallive
Copy link
Collaborator

I have been thinking about it and I think this is the only solution for that besides reporting it to Sentry itself. Loggin it through Laravel is also not possible since that would make it loop possibly.

But looking at #40 this would result in a log entry every single request in their app right?

@pedrommone
Copy link
Contributor

pedrommone commented Dec 9, 2016

@stayallive Sentry only handle exception failures, right? If I'm right, its fine to log it back but, would be nice if it send to Sentry too.

@dcramer
Copy link
Member Author

dcramer commented Dec 9, 2016

@pedrommone we pick up logs in certain situations, and also use them as breadcrumbs, which means it becomes complex and could be recursive

@dcramer
Copy link
Member Author

dcramer commented Dec 9, 2016

@stayallive yeah it'd hit repeatedly for #40, but our goal would also be to fix that -- this is mostly insurance

@pedrommone
Copy link
Contributor

pedrommone commented Dec 13, 2016

@dcramer can you PR this one and open a new issue to figure out how to log without hit repeatedly?

@dcramer
Copy link
Member Author

dcramer commented Dec 14, 2016

@pedrommone you cannot avoid repeatedly logging -- they're unique requests with no global state

@dcramer dcramer merged commit d68ae2d into master Dec 14, 2016
@dcramer dcramer deleted the feature/handle-breadcrumb-errors branch December 14, 2016 00:41
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

Successfully merging this pull request may close these issues.

5 participants