Skip to content

Convert header_sent exception into a notice error.#5614

Merged
lorenzo merged 2 commits intomasterfrom
issue-5595
Jan 9, 2015
Merged

Convert header_sent exception into a notice error.#5614
lorenzo merged 2 commits intomasterfrom
issue-5595

Conversation

@markstory
Copy link
Copy Markdown
Member

Throwing an exception here, causes an infinite loop when handling fatal errors, as the shutdown function sends headers automatically.

Refs #5595

Throwing an exception here, causes an infinite loop when handling fatal
errors, as the shutdown function sends headers automatically.

Refs #5595
@markstory markstory added this to the 2.6.1 milestone Jan 9, 2015
@markstory
Copy link
Copy Markdown
Member Author

While this does solve the infinite loop problem, fatal error pages in development mode will still show multiple notice errors as the error response page contains 3 headers that cannot be sent.

@ADmad
Copy link
Copy Markdown
Member

ADmad commented Jan 9, 2015

fatal error pages in development mode will still show multiple notice errors as the error response page contains 3 headers that cannot be sent.

I guess we can live with that.

@markstory
Copy link
Copy Markdown
Member Author

The alternatives I could come up with included:

  • no errors at all.
  • use logging instead. This adds a dependency to logging in the network code which isn't ideal.

@ADmad
Copy link
Copy Markdown
Member

ADmad commented Jan 9, 2015

I am in favor of alternative # 1 :)

@lorenzo
Copy link
Copy Markdown
Member

lorenzo commented Jan 9, 2015

👍 to alternative 1

@dereuromark
Copy link
Copy Markdown
Member

We could also add a config value for it :) App.disableHeaderWarning for all those that cannot comply with clean 2.x code and need the 100% BC here.

Not having that (useful) warning at all will be most unfortunate for the majority of 2.x users.

@ADmad
Copy link
Copy Markdown
Member

ADmad commented Jan 9, 2015

Has anyone else got a blank page due to headers being already sent as stated in the original issue #3990? If you try to redirect after headers (output) already being sent PHP itself would show an error.

After discussing it a bit, we've come to consensus that emitting errors
or throwing exceptions are both pretty disruptive and annoying to deal
with. Instead we'll revert to the previous 2.x behavior of doing
nothing.
@markstory
Copy link
Copy Markdown
Member Author

Changed up.

lorenzo added a commit that referenced this pull request Jan 9, 2015
Convert header_sent exception into a notice error.
@lorenzo lorenzo merged commit 0a4141c into master Jan 9, 2015
@lorenzo lorenzo deleted the issue-5595 branch January 9, 2015 21:25
@pyrite357
Copy link
Copy Markdown

I could go either way (no errors at all or a config var to disable this all together like @dereuromark suggested. I actually thought of that myself earlier, and didn't suggest it, but glad he did now.

I'm working on cleaning up my 2.x code, but it will take several months. It's not changing the code, but testing it afterwards that takes a long time.

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