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

getCallerFromStackTrace returns null, if stack has all vendor paths and ignore_packages is true #1072

Closed
Spice-King opened this issue Jun 3, 2021 · 5 comments · Fixed by #1128
Assignees

Comments

@Spice-King
Copy link

  • Telescope Version: 4.4.10
  • Laravel Version: 8.44.0
  • Laravel Octane Version: 1.0.3
  • Swoole Version: 4.6.7
  • PHP Version: 8.0.6
  • Database Driver & Version: MS SQL Server 12.0 on Windows, sqlsrv 5.9.0

This sucker drove me to near madness. I could not get yasd to work for me, so I was flying blind at poking around and dumping trash to my logs. It would disappear if I dragged it all back to php-fpm and xdebug to try and isolate, would only happen on some routes, and, if I'd reboot Swoole/Octane, it would come and go. Of course I messed up updating to use Octane or something, said the gremlin in my head, as I spend a day trying to figure out what I did wrong to get this random error. So after that day, I've pinned it down to truly not be directly my fault.

Description:

Telescope never accounts for the possibility that a stacktrace it generates to figure out what called a given event it watches would not have a single non-vendor file in it. Normally, public/index.php would be in there at the start of the trace, providing a failsafe entry at the end, but Octane/Swoole eliminates it from the trace, if it was even called at all. Given that my environment only shows Inertia as the one non Laravel vendor file in this stacktrace, I'd assume even with ignore_pacages set to false it could happen while just having code in the Laravel package namespace.

Here is a lightly cleaned up exception, with select bits of data scrubbed, stolen from Telescope's API endpoint for viewing them.
https://gist.github.com/Spice-King/e533e5483ff73a0451c69946f0dd17f3

Steps To Reproduce:

  1. New Laravel project
  2. Install Swoole, Octane, and Telescope
  3. Generate a model with controller, and policy
  4. Add route for controller
  5. On controller, authorize the resource for the model
  6. Set policy to allow for everything
  7. Access route for model
  8. Profit! 💥

Untested, but I'm done for the day after bashing my head on issues stemming from ripping out old stuff and adding new. I'll do up a repo case if you want it tomorrow. Octane has defiantly made my project faster (2-3x faster in TTFB), but it has caused a good number of growing pains relating to updating and replacing code that most probably won't get Octane support and was on the docket to do anyway. Thankfully, I threw it all into my staging system and poked it a lot for sanity sake, so no user saw this.

@Spice-King
Copy link
Author

Spice-King commented Jun 4, 2021

There a test case: https://github.com/Spice-King/telescope-1072

Oh yea, #804 is related, but this is not due to having Telescope enabled in unit tests.

@driesvints
Copy link
Member

MS SQL Server 12.0

We only support SQL Server 2017 and up: https://laravel.com/docs/8.x/database#introduction

@Spice-King
Copy link
Author

Test case repo demos it on SQLite, and this blows up far before SQL code anyway.

@driesvints
Copy link
Member

I'm sorry @Spice-King but I'm going to have to direct you to a support channel. This is such a custom setup (own docker setup, etc) that I can't help you with. Please try a support channel instead:

@Spice-King
Copy link
Author

Spice-King commented Jun 11, 2021

@driesvintsm, you can can skip using my docker setup, I just included it as that was my workflow. All it is is the base PHP-FPM image with Swoole installed. I just never used Sail since I made my own workflow before Sail existed, but here you go, I updated my test repo to use Sail instead, which still makes Telescope blow up with MySQL 8.0.25.

I'd just make a PR to fix this, but there is about 4 ways I can think of to go about that. Make FetchesStackTrace#getCallerFromStackTrace return the first valid frame if it failed to find one to begin with, returning the last valid one is also equally as valid. It could return a dummy frame with data that says that Telescope failed to find any usable data from the trace. Or lastly, you could mark it that the function can return null, then remember to null check everywhere you use it and substitute in some dummy values (out of all of them, this one I like the least).

And I'm aware about the exact SQL Server I'm stuck using is far out of date. Can't update it since the data I pull in my app belongs to another one that refuses to drop their SQL Server CRLs that only works on that version. As soon as we can murder that app and fully own the data it used to, we can then even think about changing the SQL Server we use. I swear upper management has funds for a party at this point for the day we do kill it with how much issues said app causes.

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

Successfully merging a pull request may close this issue.

3 participants