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

Crunz doesn't provide error log when emailing #161

Closed
kinoute opened this issue Nov 6, 2018 · 13 comments
Closed

Crunz doesn't provide error log when emailing #161

kinoute opened this issue Nov 6, 2018 · 13 comments
Labels

Comments

@kinoute
Copy link

kinoute commented Nov 6, 2018

Crunz version: 1.10.1

PHP version: 7.2.11

Operating system type and version:
Ubuntu 16.04

Description
It appeared recently. I don't get any error message by email when my script generates one. I do get an email with the first line in the body describing the job and its description, but afterwards there is nothing. I tried to run my script directly and it provides a fatal error, but crunz somewhat doesn't log it. I set log_errors & log_output to "true", and on both log files, there is no fatal error described as well. In the task log file, nothing appear, in the error log file, an error entry appears without the fatal error.

[2018-11-06 09:01:02] crunz.ERROR: job description(job command) [] []

When running my script manually, I get a fatal error:
PHP Fatal error: Uncaught ArgumentCountError: Too few arguments to function....

How to reproduce
Create a php script that will generate a fatal error. Set email_errors to true in config. Try crunz. You should get an email for the error, but inside there is nothing about the error, just "job's description (command)".

Possible Solution
No idea but I didn't have this problem before, it appeared recently, maybe after an update. Since I fortunately don't have a lot of errors in my code, I couldn't see this before recently 😄

@kinoute
Copy link
Author

kinoute commented Nov 6, 2018

Ah, I can see that on Oct 14th, I could see error's description in my emails. But the next day, after updating my dependencies, I didn't get the error's description.

Here is the update for crunz:

capture d ecran 2018-11-06 a 10 15 27

@kinoute kinoute changed the title Cruz doesn't provide error log when emailing Crunz doesn't provide error log when emailing Nov 6, 2018
@PabloKowalczyk
Copy link
Collaborator

PabloKowalczyk commented Nov 7, 2018

Hello @kinoute, interesting case.
Did you also update PHP runtime, php.ini especially? Maybe bug is related to display_error option - some time ago i have created https://github.com/PabloKowalczyk/crunz/tree/show-all-errors which enables displaying all errors, but if you run non-closure command by Crunz it will not work, but if You updated only Crunz then bug is 99,99% in Crunz.
Could you test version mentioned above and/or try to downgrade Crunz to v1.7.3 and test e-mail logging again?

EDIT: I've closed issue by accident, opened again, sorry.

@kinoute
Copy link
Author

kinoute commented Nov 7, 2018

I did updated PHP on my server but it was minor updates (like 7.2.8 > 7.2.9) and IIRC, for minor updates, php.ini remains untouched for minor updates? At least I only got a message about my php.ini being edited/updated with "major" updates, like 7.1 to 7.2, which I did months ago.

I tried on my Mac (10.11.6, PHP 7.2.8) both versions, Crunz 1.7.3 & Crunz 1.10.1, with a little script/repo and both worked correctly...

I then uploaded the repo with 1.7.3 to my server, worked. Updated composer to 1.10.1, email is again not showing the fatal error.

Here is the repo: https://github.com/kinoute/crunz-173.

Server (7.2.11):
php -i | grep 'error'
display_errors => Off => Off
display_startup_errors => Off => Off
error_append_string => no value => no value
error_log => no value => no value
error_prepend_string => no value => no value
error_reporting => 22527 => 22527
html_errors => Off => Off
ignore_repeated_errors => Off => Off
log_errors => On => On
log_errors_max_len => 1024 => 1024
track_errors => Off => Off
xmlrpc_error_number => 0 => 0
xmlrpc_errors => Off => Off
intl.error_level => 0 => 0
opcache.error_log => no value => no value

Mac (7.2.8):
display_errors => STDOUT => STDOUT
display_startup_errors => On => On
error_append_string => no value => no value
error_log => no value => no value
error_prepend_string => no value => no value
error_reporting => 32767 => 32767
html_errors => Off => Off
ignore_repeated_errors => Off => Off
log_errors => On => On
log_errors_max_len => 1024 => 1024
track_errors => Off => Off
xmlrpc_error_number => 0 => 0
xmlrpc_errors => Off => Off
intl.error_level => 0 => 0
xdebug.force_display_errors => Off => Off
xdebug.force_error_reporting => 0 => 0
xdebug.show_error_trace => Off => Off

@PabloKowalczyk
Copy link
Collaborator

So, it seems that it is bug in Crunz itself. I have done some digging and found out that PR #137 was meant to fix issues with output from stderr, but, as we can see, it fails.
I have created small patch - https://github.com/PabloKowalczyk/crunz/tree/fix-command-output could you test it? In this version whole output from Event is catched and then used for log/mail/display, i hope it fix the issue.

@kinoute
Copy link
Author

kinoute commented Nov 8, 2018

How can I use this patch with composer? Sorry, I'm bad when it comes to custom repo/fork etc.

@PabloKowalczyk
Copy link
Collaborator

@kinoute
Copy link
Author

kinoute commented Nov 8, 2018

It works on my server. As soon as I changed composer to use 1.10.1, I don't get the error (in my terminal after running schedule:run, and in the email, no error).

@PabloKowalczyk
Copy link
Collaborator

So, patched version works as expected and fixes your issue?

@kinoute
Copy link
Author

kinoute commented Nov 8, 2018

Yes.

@PabloKowalczyk
Copy link
Collaborator

Great, i will make PR and merge it later. Thanks @kinoute.

@danfsd
Copy link

danfsd commented Nov 8, 2018

Came here because of the same issue, but it was not logging on file as well. Just tested the patched version from @PabloKowalczyk and now it's working.

@PabloKowalczyk
Copy link
Collaborator

@danfsd Yep, i also fixed "log" and "display", not only "mail".

@PabloKowalczyk
Copy link
Collaborator

Merged and released as v1.11.0-beta.2, thanks guys.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

3 participants