Skip to content

Commit

Permalink
Fix disconnected session activity (#675)
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
boatbomber authored Jun 1, 2023
1 parent d87c76a commit 6b0f7f9
Show file tree
Hide file tree
Showing 2 changed files with 18 additions and 1 deletion.
17 changes: 16 additions & 1 deletion plugin/src/ApiContext.lua
Original file line number Diff line number Diff line change
Expand Up @@ -93,6 +93,7 @@ function ApiContext.new(baseUrl)
__sessionId = nil,
__messageCursor = -1,
__connected = true,
__activeRequests = {},
}

return setmetatable(self, ApiContext)
Expand All @@ -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)
Expand Down Expand Up @@ -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
Expand All @@ -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()
Expand Down
2 changes: 2 additions & 0 deletions plugin/src/ServeSession.lua
Original file line number Diff line number Diff line change
Expand Up @@ -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)

Expand Down

0 comments on commit 6b0f7f9

Please sign in to comment.