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

Handle variadic and not declared arguments in stack traces #1650

Draft
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

xwillq
Copy link

@xwillq xwillq commented Nov 25, 2023

This pull request fixes #1645.
Short summary: SDK wasn't sending some function arguments to Sentry, namely default, variadic, not declared arguments and arguments with value null. This pull request reworks function responsible for handling parameters in order to send all of the above.

I tested my changes with all inputs variations I could think of, it seems to be working. However, I don't quite understand how to add tests for it. Testing this method requires using stack trace with real function or real class and method. Where do I create them? Current FrameBuilder tests don't do anything similar, so I'm not sure what would be the right way to do it.

Also, PHPStan gives me Unreachable statement - code above always terminates. error on lines 254 and 267. Those statements are definitely reachable, and I confirmed it with manual testing. I tried changing method parameter from mixed[] to array<array-key, mixed> but it didn't help. Do you have an idea what could be causing this?

@xwillq
Copy link
Author

xwillq commented Nov 25, 2023

Creating this as draft before all mentioned issues are addressed.

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 the contribution!

To create tests for this, you can create some stub (anonimous?) classes or methods that have the kind of parameters that you need to test, and call the SOT on that.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Stack traces are missing variadic and not declared arguments
3 participants