Uh oh!
There was an error while loading.Please reload this page.
- Notifications
You must be signed in to change notification settings - Fork9.7k
[HttpKernel] Skip corrupted CSV data in FileProfilerStorage#50947
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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.
Already on GitHub?Sign in to your account
Uh oh!
There was an error while loading.Please reload this page.
Conversation
derrabus commentedJul 12, 2023
Thank you. Please pick a more meaningful PR title and add a test. |
radar3301 commentedJul 12, 2023 • edited
Loading Uh oh!
There was an error while loading.Please reload this page.
edited
Uh oh!
There was an error while loading.Please reload this page.
Done. No idea how I didn't see that.
Sorry, but I'm not going to write 50+ lines of code of unit test for such a trivial 2 LoC sanity check... |
src/Symfony/Component/HttpKernel/Profiler/FileProfilerStorage.php OutdatedShow resolvedHide resolved
Uh oh!
There was an error while loading.Please reload this page.
src/Symfony/Component/HttpKernel/Profiler/FileProfilerStorage.php OutdatedShow resolvedHide resolved
Uh oh!
There was an error while loading.Please reload this page.
fix "Undefined array key" by skipping invalid/corrupted lines
d30f689 tob4e942dComparenicolas-grekas commentedJul 13, 2023
Thank you@radar3301. |
Technically, part of this fix could go all the way back to 2.1, but since we're not sure why/how exactly the index.csv file is being corrupted in the first place, this PR should just serve as a stop-gap measure until the root cause can be identified.
A corrupted index.csv file is especially noticeable since 6.3, when the "Remove Expired Profiles" feature was added by#47352 .
RE: improves/fixes on PR#50913:
Warnings are (usually?) only escalated to errors in debug mode, so having a try/catch is a bit more overhead than I think is actually needed, and I believe just checking that the line read from the csv is not corrupted is the better/faster option. Granted, the profiler should really only be active in debug/non-production modes anyway, but "should" and reality usually don't align...
@alamirault@MatTheCat@benjaminfunk@Pelagoss@derrabus