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

Bug: Chrome logger does not work. #2616

Closed
dafriend opened this issue Feb 25, 2020 · 6 comments
Closed

Bug: Chrome logger does not work. #2616

dafriend opened this issue Feb 25, 2020 · 6 comments
Assignees
Labels
bug Verified issues on the current code behavior or pull requests that will fix them

Comments

@dafriend
Copy link
Contributor

Describe the bug
Plain and simple, doesn't work. I think the framework changed significantly since the code was first written. The event that was to send the headers doesn't exist any longer.

I discovered this some time ago and devised a fix but never made a PR on it.

Feel free to assign this one to me. It might take a little while to get time for it. I mostly need to devise some unit tests.

@dafriend dafriend added the bug Verified issues on the current code behavior or pull requests that will fix them label Feb 25, 2020
@MGatner
Copy link
Member

MGatner commented Feb 26, 2020

Appreciate that you offered to look into it. I don’t use Chrome so I’m pretty unfamiliar with the structure but... any ideas why the tests are passing even if this has been broken for a while?

@dafriend
Copy link
Contributor Author

I knew at one point, but have forgotten now.

@dafriend
Copy link
Contributor Author

I refreshed my memory about the issue because I know y'all are curious why it passes the unit test but fails in the real world.

The unit test method calls ChromeLoggerHandler::sendLogs() which puts the required header into a response object. The test then checks the current response object for the head. It's there all right. Assertion true and we're in the green.

However. here is how ChromeLoggerHandler is set up to call sendLogs()

Events::on('post_controller', [$this, 'sendLogs'], EVENT_PRIORITY_HIGH);

Except there isn't an event named 'post_controller', and a non-existent event won't be triggered. Obviously.

@MGatner
Copy link
Member

MGatner commented Feb 26, 2020

You are so right! Good catch. I’d say just try switching to post_controller_constructor and see if that magically fixes it?

@dafriend
Copy link
Contributor Author

dafriend commented Feb 26, 2020

I tried the post_controller_constructor and it wasn't the ideal solution. I cannot remember what the exact weakness was, but there was one.

I actually have... er, that is, had a solution. I've made a mess of my local repo and cannot find the final working commit. It's in there somewhere.

It would be nice to find since I tackled the @todo and worked out some fancy object formatting which users should appreciate. I also added logic so that the ChromeLogger cannot be instantiated in the production environment. We would be quite negligent to allow that to happen.

I'm on it though.

@MGatner
Copy link
Member

MGatner commented Feb 27, 2020

Fabulous. ;) I trust it to you

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Verified issues on the current code behavior or pull requests that will fix them
Projects
None yet
Development

No branches or pull requests

2 participants