-
-
Notifications
You must be signed in to change notification settings - Fork 136
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
Empty error when invoking function with incorrect type hinting #135
Comments
Actually your closure do not work at all - https://3v4l.org/bjv0n |
The issue is not that the closure is incorrect (I purposely made it this way). The issue is that crunz does not report the error. Look at my output from crunz. It does not contain any information about the error. |
You are right, output could be more "verbose". Would you like to provide a PR for this issue? |
I did some debugging and I found that I actually get the correct output when I have: // php.ini
So it seems that crunz logs an empty error if there is a |
Crunz actually do not omit errors, there is issue with writing error to // \Crunz\EventRunner
protected function handleError(Event $event)
{
// ...
$this->display($event->getProcess()->getErrorOutput());
// ...
} As you can see Crunz show error output ( |
@esetnik i improved error logging/displaying, could you test changes on https://github.com/PabloKowalczyk/crunz/tree/improve-error-logging ? |
Hi @PabloKowalczyk thank you for your work. I have not had an opportunity to test the changes yet, but I reviewed the commit and it makes sense to me. The only thing that's questionable is the formatting of the error output with |
Mentioned |
Merged and released in https://github.com/lavary/crunz/releases/tag/v1.10.0. Thanks @esetnik. |
@PabloKowalczyk I actually found that after v1.10.0 with |
@esetnik did you mean |
Yes that is correct |
I have this option enabled and everything working as expected. In your previous post, "empty output" mean empty cli console or empty file where Crunz stores error logs? |
@esetnik seems errors displaying depends on BTW could could you paste output of command |
@PabloKowalczyk thanks for the update and continued hard work. When the update becomes stable I will test the changes and let you know if it resolves things. |
@PabloKowalczyk just tested with 1.11 and it's still not working. It logs the
command i'm running inside the container
src/jobs/TestTasks.php
src/jobs/set_env.sh
src/jobs/start.sh
src/jobs/crontab
Dockerfile
php -i (from inside container)
crunz.yml
|
For a quick test I also just mocked the scheduler and am getting the correct output:
note the hi12 in the output. |
On version
produces following log entry: |
Using the configuration I provided above and running your example I get the following output:
|
Interesting, for some reason you enabled loggin to file Below configuration works for me:
|
Because it is a docker container so I want the logs to go to the container logging and not to a file on the container filesystem. |
This is default behavior, there shouldn't be need to do tricks. Does it work with |
So what I know is that there is definitely a change in the project which introduces this issue. The exact same example (identical container and configuration) I provided above works fine if I revert to v1.10.0 (or earlier) but doesn't work after upgrading to v1.11.0. Is there any relevant change to how log output is captured by the |
|
On version
everything works for me as expected. Here is my output:
Could you create repository with code (including docker, Cron config, task, php config and all other configurations) to reproduce? And to answer your questions:
Yes and no. Now whole output (
Definitely no, |
@PabloKowalczyk https://github.com/esetnik/crunz-output is a minimum reproducible example. Hopefully it can help us get to the bottom of what's going on. One thing I noticed is that my container is using |
In fact I tried using
In fact it appears that symphony 4 requires php 7.1 https://symfony.com/doc/current/reference/requirements.html so i'm not sure how you managed to get |
Finally found out what is going on, you are right, |
Confirmed fixed in fix-closure-run although the project does not run on 5.6 only 7.1+. Do you want me to open a separate issue for this? It should be made note of in the documentation at least. |
Crunz is 100% compatible with PHP 5.6 - https://travis-ci.org/lavary/crunz/jobs/484720655. |
Actually you are right about that. I was accidentally installing the project dependencies using 7.3 and then trying to use them in the 5.6 docker context. So this can be closed once your branch is merged. |
Patch merged and released as |
Example:
TestTasks.php
Output:
[2018-09-04 17:47:02] crunz.ERROR: Test schedule(object(Closure)) [] []
The text was updated successfully, but these errors were encountered: