search

anlutro / laravel-4-smart-errors

Smarter error handling for Laravel 4
65 stars 13 forks source link

Session data, user agents, and CCs. #26

Closed 501st-alpha1 closed 8 years ago

501st-alpha1 commented 8 years ago

I have a couple Laravel 4 projects that implement similar functionality as this package, so I'm looking into switching to this package to make things easier to maintain.

There are three pieces of functionality that are implemented in those projects that this package does not provide:

Would you be open to adding these items? If so, I can probably submit PRs for all three (though I may have questions about how best to implement them).

anlutro commented 8 years ago

I think all of those points seem sensible.

On Wed, 2 Sep 2015 01:50 Scott Weldon notifications@github.com wrote:

I have a couple Laravel 4 projects that implement similar functionality as this package, so I'm looking into switching to this package to make things easier to maintain.

There are three pieces of functionality that are implemented in those projects that this package does not provide:

  • Including a dump of the session, by taking Session::all() and then unseting any sensitive data.
  • Including the user agent of the client that triggered the error.
  • Sending the error notifications to an additional email address as a CC.

Would you be open to adding these items? If so, I can probably submit PRs for all three (though I may have questions about how best to implement them).

— Reply to this email directly or view it on GitHub https://github.com/anlutro/laravel-4-smart-errors/issues/26.

anlutro commented 8 years ago

I'm not sure how you would determine sensitive session data, though. With input I took the naive method of just checking if the string "password" was in the input data array key, I don't think guessing sensitive session data is as easy.

On Wed, 2 Sep 2015 07:01 Andreas Lutro anlutro@gmail.com wrote:

I think all of those points seem sensible.

On Wed, 2 Sep 2015 01:50 Scott Weldon notifications@github.com wrote:

I have a couple Laravel 4 projects that implement similar functionality as this package, so I'm looking into switching to this package to make things easier to maintain.

There are three pieces of functionality that are implemented in those projects that this package does not provide:

  • Including a dump of the session, by taking Session::all() and then unseting any sensitive data.
  • Including the user agent of the client that triggered the error.
  • Sending the error notifications to an additional email address as a CC.

Would you be open to adding these items? If so, I can probably submit PRs for all three (though I may have questions about how best to implement them).

— Reply to this email directly or view it on GitHub https://github.com/anlutro/laravel-4-smart-errors/issues/26.

501st-alpha1 commented 8 years ago

Cool, I'll get started on some PRs. My idea was to add another config option so that the user may define an array of keys that they want wiped.

anlutro commented 8 years ago

Sounds good.

anlutro commented 8 years ago

Should be completed in https://github.com/anlutro/laravel-4-smart-errors/commit/089deb403c7680ace0ddd883111f557549866c18 - @501st-alpha1 do you want to change your version requirement to dev-master and test it for a bit?

The changes will be released as 2.5.0 when I'm confident everything works as it's supposed to.

501st-alpha1 commented 8 years ago

I have this running in production for one of the projects. Just got the first notification in, and it seems to be working fine.

501st-alpha1 commented 8 years ago

I've gotten some more errors in over the past few days. I have verified that the session wiping works as expected, and the user agent is included in the email.

anlutro commented 8 years ago

Awesome - I'm out traveling so it may be a few days until I can tag a release.

On Thu, 17 Sep 2015 18:54 Scott Weldon notifications@github.com wrote:

I've gotten some more errors in over the past few days. I have verified that the session wiping works as expected, and the user agent is included in the email.

— Reply to this email directly or view it on GitHub https://github.com/anlutro/laravel-4-smart-errors/issues/26#issuecomment-141146862 .

501st-alpha1 commented 8 years ago

No prob.

anlutro commented 8 years ago

Completely forgot about this! I just tagged 2.5.0.