From 8fb977ad01e3851251f18e058f7abc0b71740527 Mon Sep 17 00:00:00 2001 From: Robert Mosolgo Date: Thu, 5 Dec 2024 13:02:30 -0500 Subject: [PATCH 1/4] Address some possible race conditions, add more debugging --- lib/graphql/subscriptions.rb | 2 ++ .../action_cable_subscriptions.rb | 2 ++ .../app/assets/javascripts/application.js | 24 ++++++++++++++++--- spec/dummy/app/views/pages/show.html | 20 +++++++++++----- 4 files changed, 39 insertions(+), 9 deletions(-) diff --git a/lib/graphql/subscriptions.rb b/lib/graphql/subscriptions.rb index a39d422d87..b1d60efa30 100644 --- a/lib/graphql/subscriptions.rb +++ b/lib/graphql/subscriptions.rb @@ -89,6 +89,8 @@ def trigger(event_name, args, object, scope: nil, context: {}) scope: scope, context: context, ) + # TODO remove this debugging + puts "[trigger-event] topic: #{event.topic} / fingerprint: #{event.fingerprint}" execute_all(event, object) end diff --git a/lib/graphql/subscriptions/action_cable_subscriptions.rb b/lib/graphql/subscriptions/action_cable_subscriptions.rb index 5c01517877..2c2980dc6d 100644 --- a/lib/graphql/subscriptions/action_cable_subscriptions.rb +++ b/lib/graphql/subscriptions/action_cable_subscriptions.rb @@ -146,6 +146,8 @@ def write_subscription(query, events) channel.stream_from(stream) @subscriptions[subscription_id] = query events.each do |event| + # TODO remove this debugging + puts "[write-subscription-event] topic: #{event.topic} / fingerprint: #{event.fingerprint}" # Setup a new listener to run all events with this topic in this process setup_stream(channel, event) # Add this event to the list of events to be updated diff --git a/spec/dummy/app/assets/javascripts/application.js b/spec/dummy/app/assets/javascripts/application.js index fa3cb17b7c..470abfb253 100644 --- a/spec/dummy/app/assets/javascripts/application.js +++ b/spec/dummy/app/assets/javascripts/application.js @@ -28,7 +28,8 @@ var receivedCallback = options.received // Unique-ish var uuid = Math.round(Date.now() + Math.random() * 100000).toString(16) - return { + var subscription = { + _subscribed: false, subscription: App.cable.subscriptions.create({ channel: "GraphqlChannel", id: uuid, @@ -41,7 +42,8 @@ console.log("Connected", query, variables) }, received: function(payload) { - console.log("received", query, variables, payload) + subscription._subscribed = true + App.logToBody("ActionCable received: " + JSON.stringify(payload)) if (payload.result) { receivedCallback(payload) } @@ -53,12 +55,27 @@ } ), trigger: function(options) { - this.subscription.perform("make_trigger", options) + if (!subscription._subscribed) { + options.retries ||= 0 + options.retries++ + if (options.retries > 5) { + throw new Error("Retried 5 times, failed to trigger: " + JSON.stringify(options)) + } else { + App.logToBody("Retrying trigger " + options.retries + " : " + JSON.stringify(options)) + setTimeout(function() { + subscription.trigger(options) + }, 500) + } + } else { + App.logToBody("Triggering " + JSON.stringify(options)) + this.subscription.perform("make_trigger", options) + } }, unsubscribe: function() { this.subscription.unsubscribe() }, } + return subscription } // Add `text` to the HTML body, for debugging @@ -67,5 +84,6 @@ var logEntry = document.createElement("p") logEntry.innerText = text bodyLog.appendChild(logEntry) + bodyLog.append("\n") } }).call(this); diff --git a/spec/dummy/app/views/pages/show.html b/spec/dummy/app/views/pages/show.html index b1357b8576..67c80fb909 100644 --- a/spec/dummy/app/views/pages/show.html +++ b/spec/dummy/app/views/pages/show.html @@ -75,28 +75,36 @@

ActionCable Test Page

fingerprintSubscriptions[key] = [] } var subs = fingerprintSubscriptions[key] + var number = subs.length + 1 var newSub = App.subscribe({ + number: number, query: "subscription fingerprint" + key + " { counterIncremented { newValue } }", variables: {}, received: function(data) { App.logToBody("received from " + key + " " + JSON.stringify(data)) if (data.result.data.counterIncremented) { - appendItem("fingerprint-updates-" + key, "update-" + newSub.number + "-value-" + data.result.data.counterIncremented.newValue) + appendItem("fingerprint-updates-" + key, "update-" + number + "-value-" + data.result.data.counterIncremented.newValue) } else { - appendItem("fingerprint-updates-" + key, "connected-" + newSub.number) + appendItem("fingerprint-updates-" + key, "connected-" + number) } } }) - newSub.number = subs.length + 1 + App.logToBody("Appending Subscription, key: " + key + ", number:" + number ) subs.push(newSub) } function triggerWithFingerprint(key) { - App.logToBody("triggering " + key) var subs = fingerprintSubscriptions[key] - var sub = subs && subs[0] - sub && sub.trigger({field: "counterIncremented", arguments: {}, value: null}) + if (!subs) { + App.logToBody("No Subscriptions found for Key: " + key + ", in: " + Object.keys(fingerprintSubscriptions)) + } + var sub = subs[0] + if (!sub) { + App.logToBody("Empty subscriptions array for key: " + key + ", in: " + Object.keys(fingerprintSubscriptions)) + } + App.logToBody("triggering " + key) + sub.trigger({field: "counterIncremented", arguments: {}, value: null}) } function unsubscribeWithFingerprint(key) { From 27cfa7b6b7983a2ddba62dcf153c2585777774d9 Mon Sep 17 00:00:00 2001 From: Robert Mosolgo Date: Thu, 5 Dec 2024 13:31:18 -0500 Subject: [PATCH 2/4] Add ActionCable Logging --- spec/dummy/test/system/action_cable_subscription_test.rb | 3 +++ 1 file changed, 3 insertions(+) diff --git a/spec/dummy/test/system/action_cable_subscription_test.rb b/spec/dummy/test/system/action_cable_subscription_test.rb index da1117fb2e..a7ce09ba3c 100644 --- a/spec/dummy/test/system/action_cable_subscription_test.rb +++ b/spec/dummy/test/system/action_cable_subscription_test.rb @@ -2,6 +2,9 @@ require "application_system_test_case" class ActionCableSubscriptionsTest < ApplicationSystemTestCase + setup do + ActionCable.server.config.logger = Logger.new(STDOUT) + end # This test covers a lot of ground! test "it handles subscriptions" do # Load the page and let the subscriptions happen From 9729b5d893ab1d08fbdd2f9f6f076438e59049d3 Mon Sep 17 00:00:00 2001 From: Robert Mosolgo Date: Thu, 5 Dec 2024 13:34:37 -0500 Subject: [PATCH 3/4] Add missing require --- lib/graphql/subscriptions/serialize.rb | 2 ++ 1 file changed, 2 insertions(+) diff --git a/lib/graphql/subscriptions/serialize.rb b/lib/graphql/subscriptions/serialize.rb index e9cd802b76..f58f282b72 100644 --- a/lib/graphql/subscriptions/serialize.rb +++ b/lib/graphql/subscriptions/serialize.rb @@ -1,5 +1,7 @@ # frozen_string_literal: true require "set" +require "ostruct" + module GraphQL class Subscriptions # Serialization helpers for passing subscription data around. From 3a91d14e97414b46a9bc46f7b676faa3f9798f6a Mon Sep 17 00:00:00 2001 From: Robert Mosolgo Date: Thu, 5 Dec 2024 13:37:41 -0500 Subject: [PATCH 4/4] Remove library debug code --- lib/graphql/subscriptions.rb | 2 -- lib/graphql/subscriptions/action_cable_subscriptions.rb | 2 -- 2 files changed, 4 deletions(-) diff --git a/lib/graphql/subscriptions.rb b/lib/graphql/subscriptions.rb index b1d60efa30..a39d422d87 100644 --- a/lib/graphql/subscriptions.rb +++ b/lib/graphql/subscriptions.rb @@ -89,8 +89,6 @@ def trigger(event_name, args, object, scope: nil, context: {}) scope: scope, context: context, ) - # TODO remove this debugging - puts "[trigger-event] topic: #{event.topic} / fingerprint: #{event.fingerprint}" execute_all(event, object) end diff --git a/lib/graphql/subscriptions/action_cable_subscriptions.rb b/lib/graphql/subscriptions/action_cable_subscriptions.rb index 2c2980dc6d..5c01517877 100644 --- a/lib/graphql/subscriptions/action_cable_subscriptions.rb +++ b/lib/graphql/subscriptions/action_cable_subscriptions.rb @@ -146,8 +146,6 @@ def write_subscription(query, events) channel.stream_from(stream) @subscriptions[subscription_id] = query events.each do |event| - # TODO remove this debugging - puts "[write-subscription-event] topic: #{event.topic} / fingerprint: #{event.fingerprint}" # Setup a new listener to run all events with this topic in this process setup_stream(channel, event) # Add this event to the list of events to be updated