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

Don't bother trying to set headers if not possible. #1194

Merged
merged 1 commit into from
Dec 31, 2019

Conversation

jpwhite4
Copy link
Member

We often get error log reports with messages like the following:

PHP Warning:  Cannot modify header information - headers already sent by (output started at /opt/xdmod/share/html/index.php:180) in /opt/xdmod/share/configuration/linker.php on line 179

These are of course completely useless and are caused by the error
handler trying to set the headers and failing. The issue is that the
orignal message that the error handler is supposed to output does not
make it to the logs.

This change ensures that the exception handler only tries to set headers
if they have not already been sent and updates the documentation to
encourage folk to look in the xdmod exceptions log.

I manually tested this by adding a throw new Exception in index.php and loading the page.

We often get error log reports with messages like the following:
```
PHP Warning:  Cannot modify header information - headers already sent by (output started at /opt/xdmod/share/html/index.php:180) in /opt/xdmod/share/configuration/linker.php on line 179
```

These are of course completely useless and are caused by the error
handler trying to set the headers and failing. The issue is that the
orignal message that the error handler is supposed to output does not
make it to the logs.

This change ensures that the exception handler only tries to set headers
if they have not already been sent and updates the documentation to
encourage folk to look in the xdmod exceptions log.
@jtpalmer jtpalmer added this to the 9.0.0 milestone Dec 19, 2019
@jtpalmer jtpalmer added the bug Bugfixes label Dec 19, 2019
@jtpalmer
Copy link
Contributor

In the case where the headers have been sent is it worthwhile to store the error somewhere and then display it to the user (possibly by outputting it directly into JavaScript in index.php) instead of outputting it directly?

@jpwhite4
Copy link
Member Author

I was think about doing something like that, but I realized that this error handler can be called from any endpoint and some output could have already been sent (with any content type). An html/json/png fragment may already have been sent and there is not going to be an easy way to correctly close the fragment and inject a syntactically valid error message.

@jpwhite4
Copy link
Member Author

It would be interesting to look at refactoring index.php to split the static and dynamic parts apart. It would then be pretty easy to handle any catastrophic bootstrap errors and display a sensible message.

@jpwhite4 jpwhite4 merged commit 4d54166 into ubccr:xdmod9.0 Dec 31, 2019
@jpwhite4 jpwhite4 deleted the headers branch December 31, 2019 14:33
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Bugfixes
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants