10000 Find The Message automatically by mvrhov · Pull Request #1 · sroze/symfony · GitHub
[go: up one dir, main page]
Skip to content

Find The Message automatically #1

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
Merged
merged 2 commits into from
Dec 24, 2017
Merged

Find The Message automatically #1

merged 2 commits into from
Dec 24, 2017

Conversation

mvrhov
Copy link
@mvrhov mvrhov commented Dec 23, 2017

We can find the Message the Handler is supposed to handle automatically.
And thus there is no need to add handles to the message_handler tag.

sroze
sroze approved these changes Dec 24, 2017
View reviewed changes
Copy link
Owner
@sroze sroze left a comment

Choose a reason for hiding this comment

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

Great idea, thanks @mvrhov! 👍
Will probably refactor a little bit later :)

@sroze sroze merged commit 06ae184 into sroze:add-message-component Dec 24, 2017
@mvrhov mvrhov deleted the autoHandle branch December 24, 2017 15:00
sroze pushed a commit that referenced this pull request Mar 19, 2018
@chalasr
bug symfony#26111 [Security] fix merge of 2.7 into 2.8 + add test cas…
e6245ba 
…e (dmaicher)

This PR was merged into the 2.8 branch.

Discussion
----------

[Security] fix merge of 2.7 into 2.8 + add test case

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

This fixes the merge mistake done in symfony@899bf99

that caused this fail with the added test case:

```
There was 1 failure:

1) Symfony\Component\Security\Tests\Http\Firewall\UsernamePasswordFormAuthenticationListenerTest::testHandleNonStringUsername with data set #1 (false)
Failed asserting that exception of type "TypeError" matches expected exception "\Symfony\Component\HttpKernel\Exception\BadRequestHttpException". Message was: "Argument 1 passed to Symfony\Component\Security\Http\ParameterBagUtils::getParameterBagValue() must be an instance of Symfony\Component\HttpFoundation\ParameterBag, instance of Symfony\Component\HttpFoundation\Request given, called in /var/www/symfony/src/Symfony/Component/Security/Http/Firewall/UsernamePasswordFormAuthenticationListener.php on line 100" at
/var/www/symfony/src/Symfony/Component/Security/Http/ParameterBagUtils.php:39
/var/www/symfony/src/Symfony/Component/Security/Http/Firewall/UsernamePasswordFormAuthenticationListener.php:100
/var/www/symfony/src/Symfony/Component/Security/Http/Firewall/AbstractAuthenticationListener.php:140
/var/www/symfony/src/Symfony/Component/Security/Http/Tests/Firewall/UsernamePasswordFormAuthenticationListenerTest.php:102
```

Original fix in 2.7: https://github.com/symfony/symfony/pull/25657/files#diff-e07c3e5653e210d017545d47c1bd7e76R111

Commits
-------

51d9008 [Security] fix merge of 2.7 into 2.8 + add test case
sroze pushed a commit that referenced this pull request Mar 19, 2018
@stof
bug symfony#26403 fix the handling of timestamp in the MongoDBSession…
1819c8d 
…Handler (hjanuschka)

This PR was submitted for the master branch but it was squashed and merged into the 4.0 branch instead (closes symfony#26403).

Discussion
----------

fix the handling of timestamp in the MongoDBSessionHandler

| Q             | A
| ------------- | ---
| Branch?       |  master
| Bug fix?      | yes
| New feature?  | no
| BC breaks?    | no
| Deprecations? | no
| Tests pass?   | yes
| License       | MIT

in the process of upgrading from 3.4 to 4.0 we stumbled upon a issue with mongo session handler.

```
[05-Mar-2018 11:12:57 Europe/Vienna] PHP Fatal error:  Uncaught Error: Call to undefined method Symfony\Component\HttpFoundation\Session\Storage\Handler\MongoDbSessionHandler::createDateTime() in /opt/APP/vendor/symfony/http-foundation/Session/Storage/Handler/MongoDbSessionHandler.php:144
Stack trace:
#0 [internal function]: Symfony\Component\HttpFoundation\Session\Storage\Handler\MongoDbSessionHandler->updateTimestamp('96d983b59f8aef8...', 'user_obj|O:9:"k...')
#1 [internal function]: session_write_close()
#2 {main}
  thrown in /opt/APP/vendor/symfony/http-foundation/Session/Storage/Handler/MongoDbSessionHandler.php on line 144
```

this PR re-add's the method, that somehow got removed in 4.0 branch, whereas the interface forces to have the implemantation.

Commits
-------

97d9ea8 fix the handling of timestamp in the MongoDBSessionHandler
sroze pushed a commit that referenced this pull request Mar 19, 2018
@fabpot
bug symfony#26443 [Fix][3.4][HttpFoundation] Fix the updating of time…
8f4a0b6 
…stamp in the MemcachedSessionHandler (Alessandro Loffredo)

This PR was merged into the 3.4 branch.

Discussion
----------

[Fix][3.4][HttpFoundation] Fix the updating of timestamp in the MemcachedSessionHandler

| Q             | A
| ------------- | ---
| Branch?       | 3.4
| Bug fix?      | yes
| New feature?  | no
| BC breaks?    | no
| Deprecations? | no
| Tests pass?   | yes
| License       | MIT

Conditions: Symfony 3.4, PHP7 and sessions handled over memcache.

Apparently `memcached::touch()` returns `false` on a subsequent call with the same parameters. Since `updateTimestamp` is used in `AbstractSessionHandler::write()`

```
public function write($sessionId, $data)
    {
        if (\PHP_VERSION_ID < 70000 && $this->prefetchData) {
            $readData = $this->prefetchData;
            $this->prefetchData = null;

            if ($readData === $data) {
                return $this->updateTimestamp($sessionId, $data);
            }
        }
...
```

the result is that `write()` will return `false` on **any subsequent request within the same second** causing the following error:

```
HP Fatal error:  Uncaught Symfony\Component\Debug\Exception\ContextErrorException: Warning: session_write_close(): Failed to write session data (user). Please verify that the current setting of session.save_path is correct (/var/lib/php/sessions) in Unknown:0
Stack trace:
#0 [internal function]: Symfony\Component\Debug\ErrorHandler->handleError(2, 'session_write_c...', 'Unknown', 0, NULL)
#1 [internal function]: session_write_close()
#2 {main}
  thrown in Unknown on line 0
```

Can be reproduced on `symfony/skeleton:3.4` adding the following code to `public/index.php` and performing two consecutive requests:
```
$session = $kernel->getContainer()->get('session');
$session->set("foo", "bar");
```

Commits
-------

d007469 fix the updating of timestamp in the MemcachedSessionHandler
sroze pushed a commit that referenced this pull request Mar 23, 2019
@nicolas-grekas
bug symfony#29226 [Messenger] Improved message when handler class doe…
4efb16b 
…s not exist (neeckeloo)

This PR was merged into the 4.2-dev branch.

Discussion
----------

[Messenger] Improved message when handler class does not exist

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

**Problem:**

When defining a non existing messenger handler class in the `services.yml` config file, we encounter this confusing error message:

```
services:
    App\Handler\NonExistentHandler:
        tags: [messenger.message_handler]
```

```
PHP Fatal error:  Uncaught Symfony\Component\Debug\Exception\FatalThrowableError: Argument 1 passed to Symfony\Component\Messenger\DependencyInjection\MessengerPass::guessHandledClasses() must be an instance of ReflectionClass, null given, called in /app/vendor/symfony/messenger/DependencyInjection/MessengerPass.php on line 93 in /app/vendor/symfony/messenger/DependencyInjection/MessengerPass.php:189
Stack trace:
    #0 /app/vendor/symfony/messenger/DependencyInjection/MessengerPass.php(93): Symfony\Component\Messenger\DependencyInjection\MessengerPass->guessHandledClasses(NULL, 'App\\Application...')
    #1 /app/vendor/symfony/messenger/DependencyInjection/MessengerPass.php(74): Symfony\Component\Messenger\DependencyInjection\MessengerPass->registerHandlers(Object(Symfony\Component\DependencyInjection\ContainerBuilder), Array)
    #2 /app/vendor/symfony/dependency-injection/Compiler/Compiler.php(95): Symfony\Component\Messenger\DependencyInjection\MessengerPass->process(Object(Symfony\Component\DependencyInjection\ContainerBuilder))
    #3 / in /app/vendor/symfony/messenger/DependencyInjection/MessengerPass.php on line 189
```

**Proposal:**

We can throw a more relevant exception (RuntimeException) in this case to help the developer to have a better understanding of the issue.

```Invalid service "App\Handler\NonExistentHandler": class "App\Handler\NonExistentHandler" does not exist.```

Commits
-------

6ab9274 Improve error message when defining messenger handler class that does not exists
sroze pushed a commit that referenced this pull request Mar 31, 2019
@fabpot
minor symfony#30679 [Messenger] Add missing information in messenger …
5ca968d 
…logs (fabpot)

This PR was merged into the 4.3-dev branch.

Discussion
----------

[Messenger] Add missing information in messenger logs

| Q             | A
| ------------- | ---
| Branch?       | master
| Bug fix?      | yes-ish
| New feature?  | yes-ish
| BC breaks?    | no
| Deprecations? | no <!-- don't forget to update UPGRADE-*.md and src/**/CHANGELOG.md files -->
| Tests pass?   | yes    <!-- please add some, will be required by reviewers -->
| Fixed tickets | n/a
| License       | MIT
| Doc PR        | n/a

When using `messenger:consume`, I get the following logs:

```
2019-03-25T11:39:05+01:00 [info] Received message "Symfony\Component\Mailer\EnvelopedMessage"
2019-03-25T11:39:05+01:00 [warning] An exception occurred while handling message "Symfony\Component\Mailer\EnvelopedMessage"
2019-03-25T11:39:05+01:00 [info] Retrying Symfony\Component\Mailer\EnvelopedMessage - retry #1.
2019-03-25T11:39:05+01:00 [info] Sending message "Symfony\Component\Mailer\EnvelopedMessage" with "Symfony\Component\Messenger\Transport\AmqpExt\AmqpTransport"
2019-03-25T11:39:06+01:00 [info] Received message "Symfony\Component\Mailer\EnvelopedMessage"
2019-03-25T11:39:06+01:00 [warning] An exception occurred while handling message "Symfony\Component\Mailer\EnvelopedMessage"
2019-03-25T11:39:06+01:00 [info] Retrying Symfony\Component\Mailer\EnvelopedMessage - retry #2.
2019-03-25T11:39:06+01:00 [info] Sending message "Symfony\Component\Mailer\EnvelopedMessage" with "Symfony\Component\Messenger\Transport\AmqpExt\AmqpTransport"
2019-03-25T11:39:09+01:00 [info] Received message "Symfony\Component\Mailer\EnvelopedMessage"
2019-03-25T11:39:09+01:00 [warning] An exception occurred while handling message "Symfony\Component\Mailer\EnvelopedMessage"
2019-03-25T11:39:09+01:00 [info] Retrying Symfony\Component\Mailer\EnvelopedMessage - retry #3.
2019-03-25T11:39:09+01:00 [info] Sending message "Symfony\Component\Mailer\EnvelopedMessage" with "Symfony\Component\Messenger\Transport\AmqpExt\AmqpTransport"
```

So, an. error occurred, but I have no idea what's going on. The exception is in the context, but the context is not displayed by default. So, this PR fixes it.

Commits
-------

20664ca [Messenger] added missing information in messenger logs
sroze pushed a commit that referenced this pull request Apr 22, 2019
@nicolas-grekas
bug symfony#31121 [HttpKernel] Fix get session when the request stack…
c009e60 
… is empty (yceruto)

This PR was merged into the 4.2 branch.

Discussion
----------

[HttpKernel] Fix get session when the request stack is empty

| Q             | A
| ------------- | ---
| Branch?       | 4.2
| Bug fix?      | yes
| New feature?  | no
| BC breaks?    | no
| Deprecations? | no
| Tests pass?   | yes
| Fixed tickets | -
| License       | MIT

This bug happen behind an exception on a kernel response event, when one collector (e.g. `RequestDataCollector`) is trying to get the request session and the request stack is currently empty.

**Reproducer**
https://github.com/yceruto/get-session-bug (`GET /`)

See logs on terminal:
```bash
Apr 15 20:29:03 |ERROR| PHP    2019-04-15T20:29:03-04:00 Call to a member function isSecure() on null
Apr 15 20:29:03 |ERROR| PHP    PHP Fatal error:  Uncaught Symfony\Component\Debug\Exception\FatalThrowableError: Call to a member function isSecure() on null in /home/yceruto/demos/getsession/vendor/symfony/http-kernel/EventListener/SessionListener.php:43
Apr 15 20:29:03 |DEBUG| PHP    Stack trace:
Apr 15 20:29:03 |DEBUG| PHP    #0 /home/yceruto/demos/getsession/vendor/symfony/http-kernel/EventListener/AbstractSessionListener.php(59): Symfony\Component\HttpKernel\EventListener\SessionListener->getSession()
Apr 15 20:29:03 |DEBUG| PHP    #1 /home/yceruto/demos/getsession/vendor/symfony/http-foundation/Request.php(707): Symfony\Component\HttpKernel\EventListener\AbstractSessionListener->Symfony\Component\HttpKernel\EventListener\{closure}()
Apr 15 20:29:03 |DEBUG| PHP    #2 /home/yceruto/demos/getsession/vendor/symfony/http-kernel/DataCollector/RequestDataCollector.php(65): Symfony\Component\HttpFoundation\Request->getSession()
Apr 15 20:29:03 |DEBUG| PHP    #3 /home/yceruto/demos/getsession/vendor/symfony/http-kernel/Profiler/Profiler.php(167): Symfony\Component\HttpKernel\DataCollector\RequestDataCollector->collect(Object(Symfony\Component\HttpFoundation\Request), Object(Symfony\Component\HttpFoundation\Respo in /home/yceruto/demos/getsession/vendor/symfony/http-kernel/EventListener/SessionListener.php on line 43
```

Friendly ping @nicolas-grekas as author of the previous PR symfony#28244

Commits
-------

d62ca37 Fix get session when the request stack is empty
sroze pushed a commit that referenced this pull request Jul 10, 2019
@fabpot
bug symfony#32036 [Messenger] improve logs (Tobion)
e02da2a 
This PR was merged into the 4.3 branch.

Discussion
----------

[Messenger] improve logs

| Q             | A
| ------------- | ---
| Branch?       | 4.3
| Bug fix?      | no
|
9219
 New feature?  | no <!-- please update src/**/CHANGELOG.md files -->
| BC breaks?    | no     <!-- see https://symfony.com/bc -->
| Deprecations? | no <!-- please update UPGRADE-*.md and src/**/CHANGELOG.md files -->
| Tests pass?   | yes    <!-- please add some, will be required by reviewers -->
| Fixed tickets |
| License       | MIT
| Doc PR        |

The logs are currently very confusing and duplicated:

- When handled sync the uncaught error it logged and displayed by the console / http error handler anyway. Currently it the warning is duplicated and useless:

```
14:26:11 WARNING   [messenger] An exception occurred while handling message "{class}": OUCH, THAT HURTS! GO TO MOM!
14:26:11 ERROR     [console] Error thrown while running command "{class}". Message: "OUCH, THAT HURTS! GO TO MOM!"

In HandleMessageMiddleware.php line 82:

  [Symfony\Component\Messenger\Exception\HandlerFailedException]
  OUCH, THAT HURTS! GO TO MOM!
```

- When handling async is was even confusing because the actual error was logged as warning and the retry (which is a good thing) was the error.

```
13:48:15 WARNING   [messenger] An exception occurred while handling message "{class}": OUCH, THAT HURTS! GO TO MOM!
13:48:15 ERROR     [messenger] Retrying {class} - retry #1.
```

Now it's must clearer and adds even context like the delay:

```
16:20:11 ERROR     [messenger] Error thrown while handling message {class}. Dispatching for retry #3 using 4000 ms delay. Error: "OUCH, THAT HURTS! GO TO MOM!"
...
16:20:15 CRITICAL  [messenger] Error thrown while handling message {class}. Removing from transport after 3 retries. Error: "OUCH, THAT HURTS! GO TO MOM!"
```

Commits
-------

2ac7027 [Messenger] improve logs
sroze pushed a commit that referenced this pull request Aug 18, 2019
@yceruto
minor symfony#33013 [ErrorRenderer] Fix timestamp on logs view (maxhe…
38ff264 
…lias)

This PR was merged into the 4.4 branch.

Discussion
----------

[ErrorRenderer] Fix timestamp on logs view

| Q             | A
| ------------- | ---
| Branch?       | 4.4
| Bug fix?      | yes
| New feature?  | no <!-- please update src/**/CHANGELOG.md files -->
| BC breaks?    | no     <!-- see https://symfony.com/bc -->
| Deprecations? | no <!-- please update UPGRADE-*.md and src/**/CHANGELOG.md files -->
| Tests pass?   | yes    <!-- please add some, will be required by reviewers -->
| Fixed tickets | -
| License       | MIT
| Doc PR        | -

This syntax raises a php exception :
```php
(new \DateTime($log['timestamp']))->format('H:i:s');
```

```
<br />
<b>Fatal error</b>:  Uncaught Exception: DateTime::__construct(): Failed to parse time string (1565179387) at position 7 (3): Unexpected character in [...][...]:4
Stack trace:
#0 [...][...](4): DateTime-&gt;__construct('1565179387')
#1 {main}
  thrown in <b>[...][...]</b> on line <b>4</b><br />
```

Commits
-------

a0ad2d0 [ErrorRenderer] Fix timestamp on logs view
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@sroze sroze sroze approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@mvrhov @sroze
0