From fecb11cba4f88d4684e2f03cbc24e3af1f96c8be Mon Sep 17 00:00:00 2001 From: Lucien Greathouse Date: Mon, 21 Jan 2019 10:57:03 -0800 Subject: [PATCH] Adjust logging and error handling in the client * HTTP responses in the error range (400+) now properly turn into errors * ROJO_EPIPHANY_DEV_CREATE now creates more verbose configuration * Default configuration values are now much more explicit * Errors that cause session termination are labeled more clearly. --- plugin/src/ApiContext.lua | 26 +++++++++++++++++++++++--- plugin/src/Components/App.lua | 3 +-- plugin/src/DevSettings.lua | 24 ++++++++++++++++++------ plugin/src/HttpResponse.lua | 2 +- plugin/src/Logging.lua | 11 +++-------- 5 files changed, 46 insertions(+), 20 deletions(-) diff --git a/plugin/src/ApiContext.lua b/plugin/src/ApiContext.lua index 30f8c8b5..f8fd2fe6 100644 --- a/plugin/src/ApiContext.lua +++ b/plugin/src/ApiContext.lua @@ -11,6 +11,14 @@ ApiContext.__index = ApiContext -- TODO: Audit cases of errors and create enum values for each of them. ApiContext.Error = { ServerIdMismatch = "ServerIdMismatch", + + -- The server gave an unexpected 400-category error, which may be the + -- client's fault. + ClientError = "ClientError", + + -- The server gave an unexpected 500-category error, which may be the + -- server's fault. + ServerError = "ServerError", } setmetatable(ApiContext.Error, { @@ -19,6 +27,18 @@ setmetatable(ApiContext.Error, { end }) +local function rejectFailedRequests(response) + if response.code >= 400 then + if response.code < 500 then + return Promise.reject(ApiContext.Error.ClientError) + else + return Promise.reject(ApiContext.Error.ServerError) + end + end + + return response +end + function ApiContext.new(baseUrl) assert(type(baseUrl) == "string") @@ -43,6 +63,7 @@ function ApiContext:connect() local url = ("%s/api/rojo"):format(self.baseUrl) return Http.get(url) + :andThen(rejectFailedRequests) :andThen(function(response) local body = response:json() @@ -102,9 +123,7 @@ function ApiContext:read(ids) local url = ("%s/api/read/%s"):format(self.baseUrl, table.concat(ids, ",")) return Http.get(url) - :catch(function(err) - return Promise.reject(err) - end) + :andThen(rejectFailedRequests) :andThen(function(response) local body = response:json() @@ -129,6 +148,7 @@ function ApiContext:retrieveMessages() return Promise.reject(err) end) + :andThen(rejectFailedRequests) :andThen(function(response) local body = response:json() diff --git a/plugin/src/Components/App.lua b/plugin/src/Components/App.lua index 913165d5..dd79be33 100644 --- a/plugin/src/Components/App.lua +++ b/plugin/src/Components/App.lua @@ -95,8 +95,7 @@ function App:render() address = address, port = port, onError = function(message) - Logging.warn("%s", tostring(message)) - Logging.trace("Session terminated due to error") + Logging.warn("Rojo session terminated because of an error:\n%s", tostring(message)) self.currentSession = nil self:setState({ diff --git a/plugin/src/DevSettings.lua b/plugin/src/DevSettings.lua index 9dcc4fca..8edf7efb 100644 --- a/plugin/src/DevSettings.lua +++ b/plugin/src/DevSettings.lua @@ -1,12 +1,22 @@ local Config = require(script.Parent.Config) +local VALUES = { + LogLevel = { + type = "IntValue", + defaultUserValue = 2, + defaultDevValue = 3, + }, +} + +local CONTAINER_NAME = "RojoDevSettings" .. Config.codename + local function getValueContainer() - return game:FindFirstChild("RojoDev-" .. Config.codename) + return game:FindFirstChild(CONTAINER_NAME) end local valueContainer = getValueContainer() -local function getValue(name) +local function getStoredValue(name) if valueContainer == nil then return nil end @@ -20,7 +30,7 @@ local function getValue(name) return valueObject.Value end -local function setValue(name, kind, value) +local function setStoredValue(name, kind, value) local object = valueContainer:FindFirstChild(name) if object == nil then @@ -37,11 +47,13 @@ local function createAllValues() if valueContainer == nil then valueContainer = Instance.new("Folder") - valueContainer.Name = "RojoDev-" .. Config.codename + valueContainer.Name = CONTAINER_NAME valueContainer.Parent = game end - setValue("LogLevel", "IntValue", getValue("LogLevel") or 2) + for name, value in pairs(VALUES) do + setStoredValue(name, value.type, value.defaultDevValue) + end end _G[("ROJO_%s_DEV_CREATE"):format(Config.codename:upper())] = createAllValues @@ -53,7 +65,7 @@ function DevSettings:isEnabled() end function DevSettings:getLogLevel() - return getValue("LogLevel") + return getStoredValue("LogLevel") or VALUES.LogLevel.defaultUserValue end return DevSettings \ No newline at end of file diff --git a/plugin/src/HttpResponse.lua b/plugin/src/HttpResponse.lua index 637f57de..039f8538 100644 --- a/plugin/src/HttpResponse.lua +++ b/plugin/src/HttpResponse.lua @@ -31,4 +31,4 @@ function HttpResponse:json() return HttpService:JSONDecode(self.body) end -return HttpResponse +return HttpResponse \ No newline at end of file diff --git a/plugin/src/Logging.lua b/plugin/src/Logging.lua index c57130b5..4b4f90af 100644 --- a/plugin/src/Logging.lua +++ b/plugin/src/Logging.lua @@ -1,7 +1,5 @@ local DevSettings = require(script.Parent.DevSettings) -local testLogLevel = nil - local Level = { Error = 0, Warning = 1, @@ -9,17 +7,14 @@ local Level = { Trace = 3, } +local testLogLevel = nil + local function getLogLevel() if testLogLevel ~= nil then return testLogLevel end - local devValue = DevSettings:getLogLevel() - if devValue ~= nil then - return devValue - end - - return Level.Info + return DevSettings:getLogLevel() end local function addTags(tag, message)