Fix serializer crash when multi_json is loaded without RubyGems activation - #128
Open
jwils wants to merge 1 commit into
Open
Fix serializer crash when multi_json is loaded without RubyGems activation#128jwils wants to merge 1 commit into
jwils wants to merge 1 commit into
Conversation
…ation
Since 8.5.2, `Serializer::MultiJson#deprecated_gem_version_loaded?`
decides between the deprecated `MultiJson` API and the new `MultiJSON`
API by consulting `Gem.loaded_specs['multi_json'].version`. When the gem
is loaded from a plain $LOAD_PATH without RubyGems activation -- e.g.
from a `bundle install --standalone` bundle or a vendored load path --
`Gem.loaded_specs` is empty, so every request fails with:
NoMethodError: undefined method 'version' for nil
Detect the API by checking for the `MultiJSON` constant instead:
multi_json 1.21.0 is exactly the version that introduced that constant,
so the check is equivalent, and it works regardless of how the gem was
loaded.
myronmarston
pushed a commit
to block/elasticgraph
that referenced
this pull request
Jul 6, 2026
…e CI part (#1293) ## Why The `run_specs_file_by_file` CI part boots RSpec once per spec file (currently 288 files) to verify every spec file can run in isolation. To avoid paying bundler's boot cost 288 times, it used `bundle install --standalone` + binstubs, which loads gems from a static $LOAD_PATH file **without activating them through RubyGems** — so `Gem.loaded_specs` is empty at runtime. That's a subtly nonstandard gem environment, and it recently bit us: elastic-transport 8.5.2+ consults `Gem.loaded_specs['multi_json']` on every request and crashed only in this CI part, forcing a version pin in #1290 (upstream fix: elastic/elastic-transport-ruby#128). ## What A small runner script (`script/rspec_file_by_file`) boots bundler and rspec-core **once**, then runs each spec file in a `fork` of that parent: - The parent never loads any ElasticGraph or spec code, so each child starts from the same clean slate as a freshly booted `rspec` process — the isolation guarantee this build part exists to check is preserved (verified: a missing-require failure and a deliberately failing spec file both still fail the run, and subsequent files don't run, matching today's fail-fast behavior). - Children inherit a normally-activated bundle, so `Gem.loaded_specs` is populated and gems behave exactly as in production. No more standalone-only failure modes. - The runner restores its invocation environment (both `ENV` and `Bundler::ORIGINAL_ENV`) after booting bundler, so children — and anything they shell out to via `Bundler.with_original_env` — observe the same environment a fresh `rspec` process would. (The first draft missed this and this build part correctly caught it, via schema_definition's `rake_tasks_spec`; all 7 env-sensitive spec files in the repo now pass under the runner locally.) ## Timing **CI (the number that matters):** this PR's `run_specs_file_by_file` job ran in **20m49s**, vs the last four runs of the current approach on main: 23m17s, 25m55s, 26m18s, 26m29s (median ~26m). That's a **~5 minute / ~20% improvement**, faster than every recent baseline run. Local measurements (M-series macOS, warm caches, ES 9.4.2, `NO_VCR=1`): | Workload | standalone (current) | `bundle exec` per file | fork runner | |---|---|---|---| | elasticgraph-support — 19 files, all unit | 4.7s | 29.4s | **3.7s** | | elasticgraph-admin — 7 files, mostly datastore integration (median of 3 alternating runs) | **23s** | ~27s | 33s | The local admin regression is a macOS-specific fork penalty on socket I/O (raw `Net::HTTP` to local ES is ~40% slower in a forked child on macOS; pure-CPU and allocation-heavy benchmarks show zero fork penalty, and the same benchmark on Linux shows ~no penalty) — consistent with the CI job getting faster while local datastore-heavy gems get slower. Something to be aware of when running the file-by-file loop locally on a Mac, but CI is where this build part runs.
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.
Problem
Since 8.5.2,
Serializer::MultiJson#deprecated_gem_version_loaded?chooses between the deprecatedMultiJsonAPI and the newMultiJSONAPI (multi_json >= 1.21.0) by consulting:Gem.loaded_specsis only populated when gems are activated through RubyGems. When gems are loaded from a plain$LOAD_PATH— e.g. abundle install --standalonebundle, or a vendored load path — the lookup returnsnil, and every request fails during response deserialization:8.5.1 and earlier were unaffected (the serializer called
::MultiJsondirectly, without consulting gem specs). We hit this in ElasticGraph's CI, which runs part of its test suite from a standalone bundle, and pinned to 8.5.1 to work around it.Reproduction
Fix
Detect which multi_json API is available by checking for the
MultiJSONconstant instead. multi_json 1.21.0 is exactly the version that introduced that constant, so the check is behaviorally equivalent to the version comparison — and it works regardless of how the gem was loaded.Testing
Gem.loaded_specsto be empty; it fails with the productionNoMethodErrorbefore this change and passes after.MultiJsonAPI) and 1.21.1 (MultiJSONAPI) loaded from a plain$LOAD_PATHwithout RubyGems activation.One note from running
rake test:unitlocally (macOS, Ruby 3.4.7): theconnection_test.rbresurrect tests fail intermittently both with and without this change (they appear to be ordering/seed-sensitive), while the serializer tests pass consistently.Related: #125 is a different symptom in the same version-detection code path; this PR doesn't address it.