Keep Rails requests and their logs working with json 3 on Rails 8.0 and older - #62
Merged
Merged
Conversation
logtail-rack 0.2.8 times responses with the monotonic clock, which Timecop doesn't freeze, so "Completed 200 OK in 0.0ms" no longer holds. The last CI run on main still resolved logtail-rack 0.2.7. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…pport 8.0 With json 3, ActiveSupport 8.0 and older raise on every direct to_json call, because their encoder passes quirks_mode: to JSON.generate. The gems below logtail-rails call to_json for headers_json (logtail-rack), params_json, backtrace_json and each JSON log line (logtail), so every request fails with ArgumentError and nothing is logged. The tests stub ActiveSupport::JSON.encode to raise the same way on every Rails version in CI. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…21 and 0.2.9 are released Points the root Gemfile and every gemfiles/*.gemfile at the json-generate branches of logtail-ruby (logtail-ruby#53) and logtail-ruby-rack (logtail-ruby-rack#28), which replace to_json with JSON.generate. At merge time, after both releases, replace this commit with logtail ~> 0.1, >= 0.1.21 and logtail-rack ~> 0.2, >= 0.2.9 in the gemspec. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
logtail-ruby#53 converts with as_json before JSON.generate, so with json 3 a Time keeps ActiveSupport's ISO 8601 format and NaN still becomes null. Plain JSON.generate would write Time#to_s and raise on NaN. The test logs both through Rails.logger with ActiveSupport's encoder raising and expects what ActiveSupport's to_json produced before. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…til 0.1.21 and 0.2.9 are released" This reverts commit 85f4056.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
With json ≥ 3 on Rails 8.0 and older (Rails 7.2 before 7.2.4), every request of a Rails app using logtail-rails fails with a 500 and nothing is logged. ActiveSupport's encoder passes
quirks_mode:toJSON.generatewheneverto_jsonis called directly, and json 3 rejects unknown options. Evidence: Rails 7.1 and 8.0 apps with json 3.0.2 answered 500 to every request, and in the new spec here a request raisesArgumentError: unknown keyword: :quirks_modefromlogtail-rack/http_request.rb:29.logtail-rails itself has no
to_jsoncall inlib. All the calls on this path are in the gems it depends on:headers_jsonof the request and response events (http_request.rb:29,http_response.rb:24), changed in Build headers_json with JSON.generate so json 3 can't fail every request logtail-ruby-rack#28;params_json(events/controller_call.rb:16),backtrace_json(events/error.rb:15) and every JSON log line (LogEntry#to_json, used bycreate_default_loggerin the test environment and withLOGTAIL_SKIP_LOGS), changed in Encode JSON with JSON.generate so json 3 under ActiveSupport 8.0 can't break logging logtail-ruby#53.So this PR adds the Rails-level regression test and, at merge time, the dependency floors:
ActiveSupport::JSON.encodestubbed to raise the way json 3 makes it raise. That covers every Rails version in CI, json 3 or not. It expects a 200, the request, controller call, render and response events with parseableheaders_jsonandparams_json, and an error event withbacktrace_json, while the app's own exception gets through unchanged. Today the logger'sArgumentErrorreplaces it.Rails.loggerwith that encoder raising, and expects what ActiveSupport'sto_jsonwrote before:"2016-09-01T12:00:00.000Z"andnull. PlainJSON.generatewould writeTime#to_sand raise on NaN; logtail-ruby#53 converts withas_jsonfirst. There is noto_jsonorJSON.generatecall in logtail-rails' ownlibto convert the same way.to_s.to_json, which goes back through ActiveSupport's encoder. The spec caught that on the TruffleRuby legs (the log level is a Symbol), and logtail-ruby#53 now converts withas_jsonbefore generating.show_exceptions = :noneon Rails 7.1+), because since Rails 7.2 the spec app'sfalsemeans:all.gemfiles/rails-7.1.gemfileandrails-8.0.gemfilekeep theirjson < 3pin. The spec app's ownrender json:hits the same ActiveSupport bug, so the stub is what covers json 3 there.Behaviour and compatibility, once the fixed gems are in:
headers_json,params_jsonandbacktrace_jsonno longer escape HTML entities, so<,>and&(and U+2028/U+2029) stay as they are instead of\u003c,\u003e,\u0026. The JSON means the same. Everything else reads as before: logtail-ruby#53 converts values withas_jsonfirst, as ActiveSupport'sto_jsondid, so times stay ISO 8601, NaN staysnulland file uploads inparams_jsonkeep their hash form.Targets the logtail-rails 0.2.15 patch release. It depends on logtail 0.1.21 (logtail-ruby#53) and logtail-rack 0.2.9 (logtail-ruby-rack#28):
TEMP: run CI against the logtail and logtail-rack branches until 0.1.21 and 0.2.9 are releasedcommit points the rootGemfileand everygemfiles/*.gemfileat theirclaude/json-generatebranches, so CI exercises the real fixes.logtail ~> 0.1, >= 0.1.21andlogtail-rack ~> 0.2, >= 0.2.9in the gemspec, so that updating logtail-rails pulls in the fixed gems.Commits and CI:
Expect real response durations in the request specsis unrelated test maintenance that every logtail-rails PR needs right now: logtail-rack 0.2.8 times responses with the monotonic clock, which Timecop doesn't freeze, so three specs expecting "in 0.0ms" fail on every CI leg. It's identical to the same commit in Stop logging "Rendering" twice per template at debug level on Rails 7.2 to 8.1 #59, so the two merge cleanly in either order.🤖 Generated with Claude Code