cakephp-error-email icon indicating copy to clipboard operation
cakephp-error-email copied to clipboard

Ignore premature headers emission

Open smarek opened this issue 6 years ago • 4 comments

Hi, thanks for your wonderful plugin, works really well :-)

One thing, I've not found possible to configure (without subclassing), is to ignore side-WarningExceptions about premature headers emission, when the content (ie. error) is output directly from controller, not from view.

Such will result in 2 additional emails being sent, generated by vendor/cakephp/cakephp/src/Http/ResponseEmitter.php

  • Unable to emit headers. Headers sent in file=/path/to/class/that/printed/output.php line=line-number
  • Cannot modify header information - headers already sent by (output started at /path/to/class/that/printed/output.php:line-number)

These are usually side-effect, not the direct reason, of error occuring.

Would it be possible to filter out these 2 emails, in situation where there are more emails queued about error itself?

Means.

  • scenario 1
    • if controller/model/component/... outputs content, this results only in those 2 emails being sent
    • this is currently handled as it should imo
  • scenario 2
    • if controller/model/component/... produces error which causes subsequently premature content output and 2 additional emails being sent (total 3 or more emails), do not send the 2 emails about error/warning being produced

Cheers

smarek avatar Feb 05 '20 11:02 smarek

Hi @smarek I'm so glad you're finding the plugin useful. I'm not sure I completely understand whats going on, are you saying you're getting this warning on every page load emailed to you whether you have other errors on the page or not? If that is the case its most likely because you have accidentally added some space before an opening <?php tag somewhere. Anytime you have output before you send headers php will complain. Fixing this issue would stop the emails if that's whats going on. And if that is indeed what is going on I don't think the emails are being sent in error they are just telling you about this issue.

This plugin itself should not be producing any output to the screen so I don't think it would be causing the warnings you're seeing by itself.

If its not the scenario outlined above do you have a simple example I can follow with a default install of cakephp to reproduce it?

ebrigham1 avatar Feb 06 '20 00:02 ebrigham1

Hi @ebrigham1

no, the library works fine, and i'm talking about specific cases, where errorneous state of application will cause premature output and php complains.

This mentioned php complaning i'd like to ignore (not get the 2 additional emails), either at all (impractical) or if there is at least one more error/warning being reported by the plugin (optimal)

of course, ignoring completely php complaining is not good idea, however when warning is issued, and you get 1 email about warning and 2 emails about php complaining, it's not very nice

smarek avatar Feb 06 '20 14:02 smarek

Hi @smarek

Well there currently isn't anything built in to throttle emails per request. If i'm understanding correctly that's what it sounds like you're interested in? I don't see any reason why it couldn't be built in so if you feel like making a pull request for that type of functionality I could review it and accept it pending testing. One thing I would be concerned about is if its limited to 1 email per request could other emails be getting discarded that may be important to debugging the issue?

Another option you could go with is just not email the "warning" level though then you would get no warnings at all which could be undesirable.

I'm still not understanding how the plugin is triggering these excess emails as in a default cakephp install if i trigger an error in controller code before it renders a view I do not get these additional emails so I still think you have a whitespace issue somewhere in your code before a php tag which is always triggering these warnings which could fix your issue if removed.

ebrigham1 avatar Feb 08 '20 15:02 ebrigham1

@ebrigham1 yes, initially the ticket was meant for discussion about best approach to handle this. However one could probably solve this with subclassing as well, but i did not attempt to do that so far.

Throttling is maybe not the best name for the feature, because we don't want to limit the number of sent emails disregarding order of warnings/errors (even though I think it's usually deterministic order)

Maybe aggregating the reports from single request would be better approach? Ie. not sent separate email for every warning/error thrown, but aggregate all the rendered emails and sent them in single e-mail (indication of errors/warnings count in the email title and as prefix of content body would be probably necessary)

And again, no, this plugin is not cause of excess emails, there is nothing that this plugin is doing, that would make any situation worse than it already is.

smarek avatar Feb 08 '20 16:02 smarek