From 6b0f7f94b60d05e4413fca3c6d7fa47bce6b257d Mon Sep 17 00:00:00 2001
From: boatbomber <zack@boatbomber.com>
Date: Thu, 1 Jun 2023 11:18:04 -0400
Subject: [PATCH] Fix disconnected session activity (#675)

When a session is disconnected, the apiContext long-polling for messages
continues until resolved/rejected. This means that even after a session
is disconnected, a message can be received and handled.

This leads to bad behavior, as the session was already cleaned up and
the message cannot be handled correctly. The instance map was cleaned up
upon disconnect, so it will warn about unapplied changes to unknown
instances. (Like #512)

It's very easy to repro:
Connect a session, disconnect it, then save a change.


https://github.com/rojo-rbx/rojo/assets/40185666/846a7269-7043-4727-9f9c-b3ac55a18a3a

-----------

This PR fixes that neatly by tracking all active requests in a map, and
cancelling their promises when we disconnect.
---
 plugin/src/ApiContext.lua   | 17 ++++++++++++++++-
 plugin/src/ServeSession.lua |  2 ++
 2 files changed, 18 insertions(+), 1 deletion(-)

diff --git a/plugin/src/ApiContext.lua b/plugin/src/ApiContext.lua
index 78f54fed..98890d4f 100644
--- a/plugin/src/ApiContext.lua
+++ b/plugin/src/ApiContext.lua
@@ -93,6 +93,7 @@ function ApiContext.new(baseUrl)
 		__sessionId = nil,
 		__messageCursor = -1,
 		__connected = true,
+		__activeRequests = {},
 	}
 
 	return setmetatable(self, ApiContext)
@@ -113,6 +114,11 @@ end
 
 function ApiContext:disconnect()
 	self.__connected = false
+	for request in self.__activeRequests do
+		Log.trace("Cancelling request {}", request)
+		request:cancel()
+	end
+	self.__activeRequests = {}
 end
 
 function ApiContext:setMessageCursor(index)
@@ -204,7 +210,7 @@ function ApiContext:retrieveMessages()
 	local url = ("%s/api/subscribe/%s"):format(self.__baseUrl, self.__messageCursor)
 
 	local function sendRequest()
-		return Http.get(url)
+		local request = Http.get(url)
 			:catch(function(err)
 				if err.type == Http.Error.Kind.Timeout then
 					if self.__connected then
@@ -216,6 +222,15 @@ function ApiContext:retrieveMessages()
 
 				return Promise.reject(err)
 			end)
+
+		Log.trace("Tracking request {}", request)
+		self.__activeRequests[request] = true
+
+		return request:finally(function(...)
+			Log.trace("Cleaning up request {}", request)
+			self.__activeRequests[request] = nil
+			return ...
+		end)
 	end
 
 	return sendRequest()
diff --git a/plugin/src/ServeSession.lua b/plugin/src/ServeSession.lua
index 12b9ffae..f77c8c74 100644
--- a/plugin/src/ServeSession.lua
+++ b/plugin/src/ServeSession.lua
@@ -285,6 +285,8 @@ end
 function ServeSession:__mainSyncLoop()
 	return self.__apiContext:retrieveMessages()
 		:andThen(function(messages)
+			Log.trace("Serve session {} retrieved {} messages", tostring(self), #messages)
+
 			for _, message in ipairs(messages) do
 				local unappliedPatch = self.__reconciler:applyPatch(message)