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

Fix handling ESI request #213

Merged
merged 1 commit into from May 16, 2019
Merged

Conversation

franmomu
Copy link
Contributor

Hi!

When the RequestListener is handling a ESI request it fails because it has no _route defined in its attributes and it returns null, so the Scope:: setTag method fails because is expecting a string.

Copy link
Collaborator

@Jean85 Jean85 left a comment

Choose a reason for hiding this comment

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

Thanks for reporting this! Can you add a test to avoid having regressions with this bug?

@franmomu
Copy link
Contributor Author

Sorry, I should have done it in the first place 😬

if (! $event->getRequest()->attributes->has('_route')) {
return;
}

$matchedRoute = $event->getRequest()->attributes->get('_route');
Copy link
Contributor Author

@franmomu franmomu May 16, 2019

Choose a reason for hiding this comment

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

I was thinking that $matchRoute must be a string because Scope::setTag is typed with string and $event->getRequest()->attributes->get('_route') returns mixed, so maybe there should be a cast in here? just to make sure it doesn't fail.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Since that depends on Symfony, we can count in it being a string, but a cast doesn't hurt, yeah.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

done 😬

@Jean85 Jean85 merged commit 84cb150 into getsentry:master May 16, 2019
@franmomu franmomu deleted the fix_esi_request branch May 16, 2019 14:06
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.

None yet

2 participants