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

[PHP Provider] Ensure that PHP-FPM workers output is streamed to Nginx #1168

Merged
merged 2 commits into from
Aug 30, 2024

Conversation

lolautruche
Copy link
Contributor

@lolautruche lolautruche commented Aug 29, 2024

This PR allows errors and warnings from PHP-FPM workers to be streamed to Nginx output. This is critical to be able to get access to the application log, which is usually streamed on php://stderr in production.

Reference: https://serverfault.com/questions/417102/nginx-not-logging-php-errors

This allows errors and warnings to be streamed in Nginx output. This is critical to be able to get access to the application log, which is usually streamed on `php://stderr` in production.
Copy link
Contributor

@coffee-cup coffee-cup left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The PR looks good. Can you please run cargo snapshot in the root to update the tests

@lolautruche
Copy link
Contributor Author

@coffee-cup Sorry, I'm a bit ignorant in Rust 😇 .
I did run cargo snapshot, but it didn't update anything:

running 0 tests

test result: ok. 0 passed; 0 failed; 0 ignored; 0 measured; 1 filtered out; finished in 0.00s

done: no snapshots to review

I'm sure I'm missing something 😅

@lolautruche
Copy link
Contributor Author

Nevermind, I figured it out :-)

@coffee-cup coffee-cup merged commit da969c0 into railwayapp:main Aug 30, 2024
98 checks passed
@lolautruche lolautruche deleted the patch-1 branch August 31, 2024 17:05
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
release/patch Author patch release
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants