Make plugin connection much more robust, with better errors

plugin/src/Api.lua

@@ -4,32 +4,43 @@ local Config = require(script.Parent.Config)
 local Promise = require(script.Parent.Promise)
 local Version = require(script.Parent.Version)
 
-local Server = {}
-Server.__index = Server
+local Api = {}
+Api.__index = Api
+
+Api.Error = {
+	ServerIdMismatch = "ServerIdMismatch",
+}
+
+setmetatable(Api.Error, {
+	__index = function(_, key)
+		error("Invalid API.Error name " .. key, 2)
+	end
+})
 
 --[[
-	Create a new Server using the given HTTP implementation and replacer.
+	Api.connect(Http) -> Promise<Api>
 
-	If the context becomes invalid, `replacer` will be invoked with a new
-	context that should be suitable to replace this one.
+	Create a new Api using the given HTTP implementation.
 
 	Attempting to invoke methods on an invalid conext will throw errors!
 ]]
-function Server.connect(http)
+function Api.connect(http)
 	local context = {
 		http = http,
 		serverId = nil,
 		currentTime = 0,
 	}
 
-	setmetatable(context, Server)
+	setmetatable(context, Api)
 
 	return context:_start()
 end
 
-function Server:_start()
-	return self:getInfo()
+function Api:_start()
+	return self.http:get("/")
 		:andThen(function(response)
+			response = response:json()
+
 			if response.protocolVersion ~= Config.protocolVersion then
 				local message = (
 					"Found a Rojo dev server, but it's using a different protocol version, and is incompatible." ..
@@ -39,7 +50,7 @@ function Server:_start()
 					"\n\nGo to https://github.com/LPGhatguy/rojo for more details."
 				):format(
 					Version.display(Config.version), Config.protocolVersion,
-					Config.expectedServerVersionString,
+					Config.expectedApiVersionString,
 					response.serverVersion, response.protocolVersion
 				)
 
@@ -53,37 +64,49 @@ function Server:_start()
 		end)
 end
 
-function Server:getInfo()
+function Api:getInfo()
 	return self.http:get("/")
 		:andThen(function(response)
 			response = response:json()
 
+			if response.serverId ~= self.serverId then
+				return Promise.reject(Api.Error.ServerIdMismatch)
+			end
+
 			return response
 		end)
 end
 
-function Server:read(paths)
+function Api:read(paths)
 	local body = HttpService:JSONEncode(paths)
 
 	return self.http:post("/read", body)
 		:andThen(function(response)
 			response = response:json()
 
+			if response.serverId ~= self.serverId then
+				return Promise.reject(Api.Error.ServerIdMismatch)
+			end
+
 			return response.items
 		end)
 end
 
-function Server:getChanges()
+function Api:getChanges()
 	local url = ("/changes/%f"):format(self.currentTime)
 
 	return self.http:get(url)
 		:andThen(function(response)
 			response = response:json()
 
+			if response.serverId ~= self.serverId then
+				return Promise.reject(Api.Error.ServerIdMismatch)
+			end
+
 			self.currentTime = response.currentTime
 
 			return response.changes
 		end)
 end
 
-return Server
+return Api

plugin/src/Main.server.lua

@@ -11,6 +11,11 @@ local Version = require(script.Parent.Version)
 	are, show them a reminder to make sure they check their server version.
 ]]
 local function checkUpgrade()
+	-- When developing Rojo, there's no use in doing version checks
+	if Config.dev then
+		return
+	end
+
 	local lastVersion = plugin:GetSetting("LastRojoVersion")
 
 	if lastVersion then
@@ -30,10 +35,7 @@ local function checkUpgrade()
 		end
 	end
 
-	-- When developing Rojo, there's no use in storing that version number.
-	if not Config.dev then
-		plugin:SetSetting("LastRojoVersion", Config.version)
-	end
+	plugin:SetSetting("LastRojoVersion", Config.version)
 end
 
 local function main()

plugin/src/Plugin.lua

@@ -1,6 +1,6 @@
 local Config = require(script.Parent.Config)
 local Http = require(script.Parent.Http)
-local Server = require(script.Parent.Server)
+local Api = require(script.Parent.Api)
 local Promise = require(script.Parent.Promise)
 local Reconciler = require(script.Parent.Reconciler)
 
@@ -26,7 +26,7 @@ function Plugin.new()
 	local self = {
 		_http = Http.new(remote),
 		_reconciler = Reconciler.new(),
-		_server = nil,
+		_api = nil,
 		_polling = false,
 	}
 
@@ -57,37 +57,58 @@ function Plugin.new()
 	return self
 end
 
-function Plugin:server()
-	if not self._server then
-		self._server = Server.connect(self._http)
+--[[
+	Clears all state and issues a notice to the user that the plugin has
+	restarted.
+]]
+function Plugin:restart()
+	warn("The server has changed since the last request, reloading plugin...")
+
+	self._reconciler:clear()
+	self._api = nil
+	self._polling = false
+end
+
+function Plugin:api()
+	if not self._api then
+		self._api = Api.connect(self._http)
 			:catch(function(err)
-				self._server = nil
+				self._api = nil
 				return Promise.reject(err)
 			end)
 	end
 
-	return self._server
+	return self._api
 end
 
 function Plugin:connect()
 	print("Testing connection...")
 
-	return self:server()
-		:andThen(function(server)
-			return server:getInfo()
-		end)
-		:andThen(function(result)
+	return self:api()
+		:andThen(function(api)
+			local ok, info = api:getInfo():await()
+
+			if not ok then
+				return Promise.reject(info)
+			end
+
 			print("Server found!")
-			print("Protocol version:", result.protocolVersion)
-			print("Server version:", result.serverVersion)
+			print("Protocol version:", info.protocolVersion)
+			print("Server version:", info.serverVersion)
+		end)
+		:catch(function(err)
+			if err == Api.Error.ServerIdMismatch then
+				self:restart()
+				return self:connect()
+			else
+				return Promise.reject(err)
+			end
 		end)
 end
 
 function Plugin:togglePolling()
 	if self._polling then
-		self:stopPolling()
-
-		return Promise.resolve(nil)
+		return self:stopPolling()
 	else
 		return self:startPolling()
 	end
@@ -95,48 +116,51 @@ end
 
 function Plugin:stopPolling()
 	if not self._polling then
-		return
+		return Promise.resolve(false)
 	end
 
 	print("Stopped polling.")
 
 	self._polling = false
 	self._label.Enabled = false
+
+	return Promise.resolve(true)
 end
 
-function Plugin:_pull(server, project, routes)
-	local items = server:read(routes):await()
+function Plugin:_pull(api, project, routes)
+	return api:read(routes)
+		:andThen(function(items)
+			for index = 1, #routes do
+				local itemRoute = routes[index]
+				local partitionName = itemRoute[1]
+				local partition = project.partitions[partitionName]
+				local item = items[index]
 
-	for index = 1, #routes do
-		local itemRoute = routes[index]
-		local partitionName = itemRoute[1]
-		local partition = project.partitions[partitionName]
-		local item = items[index]
+				local partitionRoute = collectMatch(partition.target, "[^.]+")
 
-		local partitionRoute = collectMatch(partition.target, "[^.]+")
+				-- If the item route's length was 1, we need to rename the instance to
+				-- line up with the partition's root object name.
+				--
+				-- This is a HACK!
+				if #itemRoute == 1 then
+					if item then
+						local objectName = partition.target:match("[^.]+$")
+						item.Name = objectName
+					end
+				end
 
-		-- If the item route's length was 1, we need to rename the instance to
-		-- line up with the partition's root object name.
-		--
-		-- This is a HACK!
-		if #itemRoute == 1 then
-			if item then
-				local objectName = partition.target:match("[^.]+$")
-				item.Name = objectName
+				local fullRoute = {}
+				for _, piece in ipairs(partitionRoute) do
+					table.insert(fullRoute, piece)
+				end
+
+				for i = 2, #itemRoute do
+					table.insert(fullRoute, itemRoute[i])
+				end
+
+				self._reconciler:reconcileRoute(fullRoute, item, itemRoute)
 			end
-		end
-
-		local fullRoute = {}
-		for _, piece in ipairs(partitionRoute) do
-			table.insert(fullRoute, piece)
-		end
-
-		for i = 2, #itemRoute do
-			table.insert(fullRoute, itemRoute[i])
-		end
-
-		self._reconciler:reconcileRoute(fullRoute, item, itemRoute)
-	end
+		end)
 end
 
 function Plugin:startPolling()
@@ -149,14 +173,22 @@ function Plugin:startPolling()
 	self._polling = true
 	self._label.Enabled = true
 
-	return self:server()
-		:andThen(function(server)
-			self:syncIn():await()
+	return self:api()
+		:andThen(function(api)
+			local syncOk, result = self:syncIn():await()
 
-			local project = server:getInfo():await().project
+			if not syncOk then
+				return Promise.reject(result)
+			end
+
+			local infoOk, info = api:getInfo():await()
+
+			if not infoOk then
+				return Promise.reject(info)
+			end
 
 			while self._polling do
-				local changes = server:getChanges():await()
+				local changes = api:getChanges():await()
 
 				if #changes > 0 then
 					local routes = {}
@@ -165,34 +197,57 @@ function Plugin:startPolling()
 						table.insert(routes, change.route)
 					end
 
-					self:_pull(server, project, routes)
+					local pullOk, pullResult = self:_pull(api, info.project, routes):await()
+
+					if not pullOk then
+						return Promise.reject(pullResult)
+					end
 				end
 
 				wait(Config.pollingRate)
 			end
 		end)
-		:catch(function()
+		:catch(function(err)
 			self:stopPolling()
+
+			if err == Api.Error.ServerIdMismatch then
+				self:restart()
+				return self:startPolling()
+			else
+				return Promise.reject(err)
+			end
 		end)
 end
 
 function Plugin:syncIn()
 	print("Syncing from server...")
 
-	return self:server()
-		:andThen(function(server)
-			local project = server:getInfo():await().project
+	return self:api()
+		:andThen(function(api)
+			local ok, info = api:getInfo():await()
+
+			if not ok then
+				return Promise.reject(info)
+			end
 
 			local routes = {}
 
-			for name in pairs(project.partitions) do
+			for name in pairs(info.project.partitions) do
 				table.insert(routes, {name})
 			end
 
-			self:_pull(server, project, routes)
+			self:_pull(api, info.project, routes)
 
 			print("Sync successful!")
 		end)
+		:catch(function(err)
+			if err == Api.Error.ServerIdMismatch then
+				self:restart()
+				return self:syncIn()
+			else
+				return Promise.reject(err)
+			end
+		end)
 end
 
 return Plugin

plugin/src/Promise.lua

@@ -221,15 +221,11 @@ function Promise:await()
 		local ok = bindable.Event:Wait()
 		bindable:Destroy()
 
-		if not ok then
-			error(tostring(result[1]), 2)
-		end
-
-		return unpack(result)
+		return ok, unpack(result)
 	elseif self._status == Promise.Status.Resolved then
-		return unpack(self._value)
+		return true, unpack(self._value)
 	elseif self._status == Promise.Status.Rejected then
-		error(tostring(self._value[1]), 2)
+		return false, unpack(self._value)
 	end
 end
 

plugin/src/Reconciler.lua

@@ -124,6 +124,13 @@ function Reconciler:_reify(item)
 	return rbx
 end
 
+--[[
+	Clears any state that the Reconciler has, effectively restarting it.
+]]
+function Reconciler:clear()
+	self._routeMap:clear()
+end
+
 --[[
 	Apply the changes represented by the given item to a Roblox object that's a
 	child of the given instance.

plugin/src/RouteMap.lua

@@ -71,8 +71,8 @@ function RouteMap:clear()
 	self._map = {}
 	self._reverseMap = {}
 
-	for object in pairs(self._connectionsByRbx) do
-		object:Disconnect()
+	for _, connection in pairs(self._connectionsByRbx) do
+		connection:Disconnect()
 	end
 
 	self._connectionsByRbx = {}