Skip to content

[Messenger] Fix TraceableMessageBus implementation so it can compute caller even when used within a callback #43781

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

Conversation

Ocramius
Copy link
Contributor

Q A
Branch? 4.4
Bug fix? yes
New feature? no
Deprecations? no
Tickets
License MIT
Doc PR

Quoting commits:

This test shows that `array_map([new TraceableMessageBus(...), 'dispatch'], $messages)` does not work at all,
because `debug_backtrace()` is used in internals to compute the call-site, and it is not considering the fact
that the caller may be:

 * an internal PHP function
 * a callback (this example test scenario)
 * generated code (`eval()` result)

The output of such a failure:

```
1) Symfony\Component\Messenger\Tests\TraceableMessageBusTest::testItTracesExceptionsWhenMessageBusIsFiredFromArrayCallback
Undefined array key "line"

/app/src/Symfony/Component/Messenger/TraceableMessageBus.php:66
/app/src/Symfony/Component/Messenger/TraceableMessageBus.php:36
/app/src/Symfony/Component/Messenger/Tests/TraceableMessageBusTest.php:175
```

To be on the safe side, also removed `compact()` usage, which was making everything
much muddier and harder to comprehend.

@Ocramius Ocramius requested a review from sroze as a code owner October 27, 2021 13:32
@carsonbot carsonbot added this to the 4.4 milestone Oct 27, 2021
Copy link
Member

@derrabus derrabus left a comment

Choose a reason for hiding this comment

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

Thank you. LGTM, once the two issues fabbot found have been fixed.

@carsonbot carsonbot changed the title Fix TraceableMessageBus implementation so it can compute caller even when used within a callback [Messenger] Fix TraceableMessageBus implementation so it can compute caller even when used within a callback Oct 27, 2021
@Ocramius Ocramius force-pushed the fix/correct-backtrace-assumptions-in-traceable-message-bus-debug-backtrace branch from de2d2c3 to f534166 Compare October 27, 2021 14:52
@Ocramius
Copy link
Contributor Author

@derrabus all good according to automation now.

@nicolas-grekas nicolas-grekas force-pushed the fix/correct-backtrace-assumptions-in-traceable-message-bus-debug-backtrace branch from f534166 to 1423229 Compare October 27, 2021 16:01
@nicolas-grekas
Copy link
Member

Thank you @Ocramius.

@nicolas-grekas nicolas-grekas merged commit 36f8fe5 into symfony:4.4 Oct 27, 2021
@Ocramius Ocramius deleted the fix/correct-backtrace-assumptions-in-traceable-message-bus-debug-backtrace branch October 27, 2021 19:22
This was referenced Oct 29, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants