search

MiniProfiler / rack-mini-profiler

Profiler for your development and production Ruby rack apps.
MIT License
3.69k stars 401 forks source link

Fix stackprof not installed error #547

Closed gmcgibbon closed 1 year ago

gmcgibbon commented 1 year ago

Fixes malformed response when pp=flamegraph is used and stackprof is not installed (this happens on web servers such as puma due to the expectation that the body is an array, it crashes here: https://github.com/puma/puma/blob/d8765e362dd9b273635facefa0911252abdb584b/lib/puma/request.rb#L153).

2023-02-07 15:15:34 -0600 Read: #<NoMethodError: undefined method `each' for "Please install the stackprof gem and require it: add gem 'stackprof' to your Gemfile":String>

Also forwards headers and status code from app response error cases so that app logs are consistent with what is actually returned by middleware.

I couldn't test this through integration tests, so I had to use units. If we don't see value in the well-formed response tests, I can remove them.

codecov-commenter commented 1 year ago

Codecov Report

Merging #547 (676bf44) into master (e1d1762) will decrease coverage by 60.57%. The diff coverage is n/a.

:exclamation: Current head 676bf44 differs from pull request most recent head 9f5a083. Consider uploading reports for the commit 9f5a083 to get more accurate results

:mega: This organization is not using Codecov’s GitHub App Integration. We recommend you install it so Codecov can continue to function properly for your repositories. Learn more

@@             Coverage Diff             @@
##           master     #547       +/-   ##
===========================================
- Coverage   87.39%   26.82%   -60.57%     
===========================================
  Files          18       14        -4     
  Lines        1269      697      -572     
===========================================
- Hits         1109      187      -922     
- Misses        160      510      +350

see 17 files with indirect coverage changes

:mega: We’re building smart automated test selection to slash your CI/CD build times. Learn more

gmcgibbon commented 1 year ago

Looks like CI errors are due to change in behaviour from the redis client. There's a fix on https://github.com/MiniProfiler/rack-mini-profiler/pull/543 to get CI green.

nateberkopec commented 1 year ago

Needs a rebase and then we're set.