Skip to content

[Monolog Bridge] ChromeHeadless ChromePHPLogger support #22264

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.

Sign up for GitHub

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

Jump to bottom
Closed
redthor opened this issue Apr 4, 2017 · 1 comment
Closed

[Monolog Bridge] ChromeHeadless ChromePHPLogger support #22264

redthor opened this issue Apr 4, 2017 · 1 comment
Labels
Bug MonologBridge Status: Needs Review

Comments

@redthor
Copy link
Contributor

redthor commented Apr 4, 2017

Q A
Bug report? yes
Feature request? no
BC Break report? no
RFC? no
Symfony version 3.2

I have logged Seldaek/monolog#966 about getting Chromium 57 user agent string support. Unfortunately the regex is overridden by the bridge in the child class.

I can change the Symfony Monolog Bridge like so:

index 2636bc3..1f99552 100644
--- a/src/Symfony/Bridge/Monolog/Handler/ChromePhpHandler.php
+++ b/src/Symfony/Bridge/Monolog/Handler/ChromePhpHandler.php
@@ -41,7 +41,7 @@ class ChromePhpHandler extends BaseChromePhpHandler
             return;
         }
 
-        if (!preg_match('{\b(?:Chrome/\d+(?:\.\d+)*|Firefox/(?:4[3-9]|[5-9]\d|\d{3,})(?:\.\d)*)\b}', $event->getRequest()->headers->get('User-Agent'))) {
+        if (!preg_match(self::USER_AGENT_REGEX, $event->getRequest()->headers->get('User-Agent'))) {
             $this->sendHeaders = false;
             $this->headers = array();

However to get this patch to work I'd also need to bump the monolog minimum version from ~1.11 to ~1.19.0 (April 2016) to get the self::USER_AGENT_REGEX const in the parent class.

Otherwise I have to repeat that same change here as Seldaek/monolog#966
@fabpot
Copy link
Member

fabpot commented Apr 4, 2017

I would be very pragmatic here and just copy/paste the changes you made to Monolog for 2.7 (as a bug fix). And then, another PR to bump the min value and use the regex from Monolog in master.

@redthor redthor mentioned this issue Apr 5, 2017
Merged
redthor added a commit to redthor/symfony that referenced this issue Apr 5, 2017
@redthor
Bump monolog to 1.19 and use the agent regex const from parent, see s…
69f9586 
…ymfony#22264
@redthor redthor mentioned this issue Apr 5, 2017
Merged
fabpot added a commit that referenced this issue Apr 5, 2017
@fabpot
bug #22292 Fixes #22264 - add support for Chrome headless (redthor)
cb49907 
This PR was merged into the 2.7 branch.

Discussion
----------

Fixes #22264 - add support for Chrome headless

| Q             | A
| ------------- | ---
| Branch?       | 2.7
| Bug fix?      | yes
| New feature?  | no
| BC breaks?    | no
| Deprecations? | no
| Tests pass?   | yes
| Fixed tickets | #22264
| License       | MIT
| Doc PR        |

Commits
-------

a03dbd2 Fixes #22264 - add support for Chrome headless, see also Seldaek/monolog#966
fabpot added a commit that referenced this issue Apr 5, 2017
@fabpot
Merge branch '2.7' into 2.8
f92ada8 
* 2.7:
  Fixes #22264 - add support for Chrome headless, see also Seldaek/monolog#966
  Dont call sprintf() when no placeholders are used
fabpot added a commit that referenced this issue Apr 5, 2017
@fabpot
Merge branch '2.8' into 3.2
f53e81d 
* 2.8:
  Fixes #22264 - add support for Chrome headless, see also Seldaek/monolog#966
  bumped Symfony version to 2.8.20
  updated VERSION for 2.8.19
  updated CHANGELOG for 2.8.19
  Dont call sprintf() when no placeholders are used
@fabpot fabpot closed this as completed in a03dbd2 Apr 5, 2017
fabpot added a commit that referenced this issue Apr 5, 2017
@fabpot
Merge branch '3.2'
3aeac5a 
* 3.2:
  bumped Symfony version to 3.2.8
  updated VERSION for 3.2.7
  updated CHANGELOG for 3.2.7
  Fixes #22264 - add support for Chrome headless, see also Seldaek/monolog#966
  [Workflow] update documentation URL in readme
  bumped Symfony version to 2.8.20
  updated VERSION for 2.8.19
  updated CHANGELOG for 2.8.19
  Dont call sprintf() when no placeholders are used
fabpot added a commit that referenced this issue Apr 5, 2017
@fabpot
feature #22296 Bump monolog to 1.19 and use the agent regex const fro…
b65ebc7 
…m parent (redthor)

This PR was merged into the 3.3-dev branch.

Discussion
----------

Bump monolog to 1.19 and use the agent regex const from parent

See #22264

| Q             | A
| ------------- | ---
| Branch?       | master
| Bug fix?      | no
| New feature?  | no
| BC breaks?    | no
| Deprecations? | no
| Tests pass?   | yes
| Fixed tickets | refer to #22264
| License       | MIT
| Doc PR        |

Bump monolog and use const from parent. See #22264 (comment)

I went for `static` instead of `self` in case anyone wanted to easily override just the agent string.

Commits
-------

69f9586 Bump monolog to 1.19 and use the agent regex const from parent, see #22264
This was referenced May 1, 2017
Merged
Merged
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
Bug MonologBridge Status: Needs Review
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
4 participants
@fabpot @redthor @xabbuh @carsonbot