Fix GoogleCredentialCache raising on a Hash-shaped json_key - #8
Fix GoogleCredentialCache raising on a Hash-shaped json_key#8Clark-Thanx wants to merge 1 commit into
Conversation
fetch_fresh_token wraps json_key in StringIO before handing it to Google::Auth::ServiceAccountCredentials, which requires a String. Any app whose json_key comes from a native json/jsonb DB column gets it back as an already-deserialized Hash, so every real send through such an app raised TypeError: no implicit conversion of Hash into String. Coerce to JSON text only when it isn't already a String.
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (2)
Included review availability: 3 reviews are currently available. Your included PR review attempts over the past 7 days set your current allowance at 4 reviews per hour. 📝 WalkthroughWalkthrough
ChangesGoogle credential handling
Estimated code review effort: 2 (Simple) | ~10 minutes Merge Risk: ⚪ Minimal · up to This localized fix converts Hash-shaped credential data to JSON text while preserving String inputs, with targeted regression coverage; no actionable merge-blocking risk remains after normal checks and review. 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches 💡 1📝 Generate docstrings 💡
🧪 Generate unit tests (beta)
Comment |
Summary
GoogleCredentialCache#fetch_fresh_tokenwrapsjson_keyinStringIO.new(json_key)before handing it toGoogle::Auth::ServiceAccountCredentials, which requires a String.Fcm::Appwhosejson_keyis backed by a nativejson/jsonbDB column gets it back from ActiveRecord as an already-deserialized Hash on every read — so every real send through such an app raisedTypeError: no implicit conversion of Hash into String.json_key, so the Hash-shaped path had never actually been exercised until now (Thanx's ORD-867 rollout, which wires a real per-merchantjson_keyfor the first time).Fix: coerce to JSON text only when
json_keyisn't already a String.Test plan
spec/unit/daemon/google_credential_cache_spec.rb— reproduces the exact production error (TypeError: no implicit conversion of Hash into String, same line as the Sentry stack trace) against the unfixed code, passes after the fix.json_keycase to confirm no regression there.spec/unit/daemon/fcm/,spec/unit/client/active_record/fcm/): 38 examples, 0 failures, 1 pre-existing pending (unrelated).functional/fcm_spec.rb/fcm_priority_spec.rbfailures are pre-existing and unrelated (adatabase_cleanergem vs. Rails 7.1ActiveRecord::SchemaMigrationincompatibility in the test harness) — reproduced identically against a fresh test DB with the fix applied.🤖 Generated with Claude Code
Summary by CodeRabbit
Bug Fixes
Tests