Fix stackprof not installed error #547
Conversation
Codecov Report
📣 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 📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more |
|
Looks like CI errors are due to change in behaviour from the redis client. There's a fix on #543 to get CI green. |
spec/lib/profiler_spec.rb
Outdated
|
|
||
| expect(100..599).to include(status) | ||
| expect(headers.keys).to(all(be_a(String))) | ||
| expect(body).to(all(be_a(String))) |
There was a problem hiding this comment.
Rather than check for an Enumerable of Strings, lets do the Rack-spec thing and check for response to each?
There was a problem hiding this comment.
Done. This caught a bug in the memory profiler spec, so thanks for that!
gmcgibbon
force-pushed
the
fix_no_stackprof_error
branch
from
8a1285c
to
641495b
Compare
|
Needs a rebase and then we're set. |
Fixes malformed response when pp=flamegraph is used and stackprof is not installed. Also forwards headers and status code from app response error cases so that app logs are consistent with what is actually returned by middleware.
gmcgibbon
force-pushed
the
fix_no_stackprof_error
branch
from
641495b
to
9f5a083
Compare
Fixes malformed response when
pp=flamegraphis 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).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.