Skip to content

Comments

Fix: Invalid JSON on gc-stats (#1801)#4

Open
MitchLewis930 wants to merge 1 commit intopr_054_beforefrom
pr_054_after
Open

Fix: Invalid JSON on gc-stats (#1801)#4
MitchLewis930 wants to merge 1 commit intopr_054_beforefrom
pr_054_after

Conversation

@MitchLewis930
Copy link

PR_054

* Fix: Invalid JSON on gc-stats

Credits of this patch should be for @jdsundberg, who reported the issue
but never sent a patch. Link to his bug report is below.

Closes puma#1687.

* Update tests so JSON response is correctly parsed

JSON without a whitespace between key and values wasn't being correctly
parsed (for example: `key: 'value'` was processed ok, but `key:'value'`
wasn't).
@greptile-apps
Copy link

greptile-apps bot commented Jan 30, 2026

Greptile Overview

Greptile Summary

Fixed invalid JSON generation in the /gc-stats endpoint by replacing manual string concatenation with Ruby's .to_json method.

Key Changes:

  • lib/puma/app/status.rb: Added require 'json' and replaced manual JSON construction with GC.stat.to_json
  • test/test_cli.rb: Updated test to use JSON.parse instead of regex parsing for the second gc-stats assertion

Issue Fixed:
The old implementation manually constructed JSON strings like "{" + GC.stat.map { |k, v| "\"#{k}\": #{v}" }.join(",") + "}", which produced invalid JSON when GC.stat values were strings (they would be unquoted) or other non-numeric types.

Minor Issue:
The test file update is inconsistent - it only converted the second gc-stats parsing to use JSON.parse (line 199) while leaving the first one (lines 179-185) with the old regex-based approach. Both should use JSON.parse for consistency.

Confidence Score: 4/5

  • Safe to merge with minor test inconsistency that should be addressed
  • The core fix correctly addresses the invalid JSON issue by using the proper .to_json method. However, the test update is inconsistent (only one of two JSON parsing blocks was updated), which slightly reduces confidence
  • test/test_cli.rb should have both gc-stats parsing blocks updated to use JSON.parse for consistency

Important Files Changed

Filename Overview
lib/puma/app/status.rb Replaced manual JSON construction with proper to_json method to fix invalid JSON output when GC.stat contains non-numeric values
test/test_cli.rb Partially updated test to use JSON.parse for the second gc-stats check, but inconsistently left the first check using manual regex parsing

Sequence Diagram

sequenceDiagram
    participant Client
    participant Status as Puma::App::Status
    participant GC as Ruby GC
    participant JSON as JSON Library

    Client->>Status: GET /gc-stats
    Status->>GC: GC.stat
    GC-->>Status: Hash of GC statistics
    Status->>JSON: .to_json
    JSON-->>Status: Valid JSON string
    Status-->>Client: 200 OK with JSON response

    Note over Status,JSON: Old implementation manually<br/>constructed JSON strings,<br/>causing invalid JSON for<br/>non-numeric values

    Note over Status,JSON: New implementation uses<br/>to_json for proper<br/>escaping and formatting
Loading

Copy link

@greptile-apps greptile-apps bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

2 files reviewed, 1 comment

Edit Code Review Agent Settings | Greptile

@greptile-apps
Copy link

greptile-apps bot commented Jan 30, 2026

Additional Comments (1)

test/test_cli.rb
Inconsistent JSON parsing - this first gc-stats check still uses manual regex parsing while the second check (line 199) uses JSON.parse. Both should use JSON.parse for consistency.

    gc_stats = JSON.parse(json_line)
    gc_count_before = gc_stats["count"].to_i

Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!

Prompt To Fix With AI
This is a comment left during a code review.
Path: test/test_cli.rb
Line: 179:185

Comment:
Inconsistent JSON parsing - this first gc-stats check still uses manual regex parsing while the second check (line 199) uses `JSON.parse`. Both should use `JSON.parse` for consistency.

```suggestion
    gc_stats = JSON.parse(json_line)
    gc_count_before = gc_stats["count"].to_i
```

<sub>Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!</sub>

How can I resolve this? If you propose a fix, please make it concise.

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.

2 participants