Add Retries and Send Delay

lua/gm_express/sh_helpers.lua

@@ -2,6 +2,7 @@ AddCSLuaFile()
 express.version = 1
 express.revision = 1
 express._putCache = {}
+express._maxCacheTime = ( 24 - 1 ) * 60 * 60 -- TODO: Get this from the server, similar to the version check
 express._waitingForAccess = {}
 express.domain = CreateConVar(
     "express_domain", "gmod.express", FCVAR_ARCHIVE + FCVAR_REPLICATED, "The domain of the Express server"
@@ -13,6 +14,10 @@ express.domain_cl = CreateConVar(
     "express_domain_cl", "", FCVAR_ARCHIVE + FCVAR_REPLICATED, "The client-specific domain of the Express server. If empty, express_domain will be used."
 )
 
+express.sendDelay = CreateConVar(
+    "express_send_delay", 0.15, FCVAR_ARCHIVE + FCVAR_REPLICATED, "How long to wait (in seconds) before sending the Express Message ID to the recipient (longer delays will result in increased reliability)"
+)
+
 
 -- Runs the correct net Send function based on the realm --
 function express.shSend( target )
@@ -121,7 +126,7 @@ function express:_getSize( id, cb )
 end
 
 
--- Encodes and compresses the given data, then sends it to the API if not already cached --
+---Encodes and compresses the given data, then sends it to the API if not already cached
 function express:_put( data, cb )
     if table.Count( data ) == 0 then
         error( "Express: Tried to send empty data!" )
@@ -141,18 +146,22 @@ function express:_put( data, cb )
 
     local hash = util.SHA1( data )
 
-    local cachedId = self._putCache[hash]
-    if cachedId then
-        -- Force the callback to run asynchronously for consistency
-        timer.Simple( 0, function()
-            cb( cachedId, hash )
-        end )
+    local cached = self._putCache[hash]
+    if cached then
+        local cachedAt = cached.cachedAt
+
+        if os.time() <= ( cachedAt + self._maxCacheTime ) then
+            -- Force the callback to run asynchronously for consistency
+            timer.Simple( 0, function()
+                cb( cached.id, hash )
+            end )
 
-        return
+            return
+        end
     end
 
     local function wrapCb( id )
-        self._putCache[hash] = id
+        self._putCache[hash] = { id = id, cachedAt = os.time() }
         cb( id, hash )
     end
 
@@ -166,20 +175,33 @@ function express:_put( data, cb )
 end
 
 
--- Forwards the given parameters to the putter function, then alerts the recipient --
-function express:_send( message, data, plys, onProof )
-    self:_put( data, function( id, hash )
-        net.Start( "express" )
-        net.WriteString( message )
-        net.WriteString( id )
-        net.WriteBool( onProof ~= nil )
-
+-- TODO: Fix GLuaTest so we can actually test this function...
+-- Creates a contextual callback for the :_put endpoint, delaying the notification to the recipient(s) --
+function express:_putCallback( message, plys, onProof )
+    return function( id, hash )
         if onProof then
             self:SetExpected( hash, onProof, plys )
         end
 
-        express.shSend( plys )
-    end )
+        -- Cloudflare isn't fulfilling their promise that the first lookup in
+        -- each region will "search" for the target key in K/V if it has't been cached yet.
+        -- This delay makes it more likely that the data will have "settled" into K/V before the first lookup
+        -- (Once it's cached as a 404, it'll stay that way for about 60 seconds)
+        timer.Simple( self.sendDelay:GetFloat(), function()
+            net.Start( "express" )
+            net.WriteString( message )
+            net.WriteString( id )
+            net.WriteBool( onProof ~= nil )
+
+            express.shSend( plys )
+        end )
+    end
+end
+
+
+-- Calls the _put function with a contextual callback --
+function express:_send( message, data, plys, onProof )
+    self:_put( data, self:_putCallback( message, plys, onProof ) )
 end
 
 

lua/gm_express/sh_init.lua

@@ -32,13 +32,23 @@ end
 
 
 -- Retrieves and parses the data for given ID --
-function express:Get( id, cb )
+function express:Get( id, cb, _attempts )
+    _attempts = _attempts or 0
     local url = self:makeAccessURL( "read", id )
 
     local success = function( code, body )
-        express._checkResponseCode( code )
+        if code == 404 then
+            assert( _attempts <= 25, "express:Get() failed to retrieve data after 25 attempts: " .. id )
+            timer.Simple( 0.125 * _attempts, function()
+                self:Get( id, cb, _attempts + 1 )
+            end )
+            return
+        end
 
-        local hash = util.SHA1( body )
+        express._checkResponseCode( code )
+        if _attempts > 0 then
+            print( "express:Get() succeeded after " .. _attempts .. " attempts: " .. id )
+        end
 
         if string.StartWith( body, "<enc>" ) then
             body = util.Decompress( string.sub( body, 6 ) )
@@ -47,6 +57,7 @@ function express:Get( id, cb )
             end
         end
 
+        local hash = util.SHA1( body )
         local decodedData = pon.decode( body )
         cb( decodedData, hash )
     end

lua/tests/gm_express/sh_helpers.lua

@@ -499,7 +499,10 @@ return {
                 local mockCallback = stub()
                 local putStub = stub( express, "Put" )
 
-                express._putCache[mockHash] = mockId
+                express._putCache[mockHash] = {
+                    id = mockId,
+                    cachedAt = os.time()
+                }
 
                 stub( pon, "encode" ).returns( "encoded-data" )
                 stub( util, "Compress" ).returns( mockData )
@@ -543,7 +546,10 @@ return {
 
                 expect( putStub ).was.called()
                 expect( mockCallback ).was.called()
-                expect( express._putCache[mockHash] ).to.equal( mockId )
+
+                local actualCached = express._putCache[mockHash]
+                expect( actualCached ).to.exist()
+                expect( actualCached.id ).to.equal( mockId )
             end,
 
             cleanup = function( state )
@@ -592,43 +598,19 @@ return {
 
         -- express:_send
         {
-            name = "express._send calls _put with a callback that nets message info and does not set expected if no onProof was provided",
-            func = function()
-                stub( net, "Start" )
-                stub( net, "WriteString" )
-                stub( net, "WriteBool" )
-
-                local setExpected = stub( express, "SetExpected" )
-                local shSend = stub( express, "shSend" )
-                local putStub = stub( express, "_put" ).with( function( _, _, cb )
-                    cb( "test-id", "test-hash" )
-                end )
-
-                express:_send( "test-message", "test-data", {} )
-
-                expect( putStub ).was.called()
-                expect( setExpected ).wasNot.called()
-                expect( shSend ).was.called()
-            end
-        },
-        {
-            name = "express._send calls _put with a callback that nets message info and sets expected if onProof was provided",
+            name = "express._send calls _putCallback",
             func = function()
-                stub( net, "Start" )
-                stub( net, "WriteString" )
-                stub( net, "WriteBool" )
+                local putCallback = stub()
+                stub( express, "_putCallback" ).returns( putCallback )
 
-                local setExpected = stub( express, "SetExpected" )
-                local shSend = stub( express, "shSend" )
                 local putStub = stub( express, "_put" ).with( function( _, _, cb )
                     cb( "test-id", "test-hash" )
                 end )
 
                 express:_send( "test-message", "test-data", {}, stub() )
 
                 expect( putStub ).was.called()
-                expect( setExpected ).was.called()
-                expect( shSend ).was.called()
+                expect( putCallback ).was.called()
             end
         },