From a4a26fcec81d12a21bdcf8b0af7736fdb8d0d9ff Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Miroslav=20Bajto=C5=A1?= <oss@bajtos.net>
Date: Thu, 7 Mar 2024 16:44:34 +0100
Subject: [PATCH] fixup! improve error handling
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

Signed-off-by: Miroslav Bajtoš <oss@bajtos.net>
---
 lib/miner-lookup.js       |  9 +++++++--
 lib/spark.js              | 36 +++++++++++++++++++++++++++++-------
 test/miner-lookup.test.js |  2 +-
 3 files changed, 37 insertions(+), 10 deletions(-)

diff --git a/lib/miner-lookup.js b/lib/miner-lookup.js
index 70355a5..d6be8ee 100644
--- a/lib/miner-lookup.js
+++ b/lib/miner-lookup.js
@@ -5,8 +5,13 @@ import { RPC_REQUEST } from './constants.js'
  * @returns {Promise<string>} Miner's PeerId, e.g. `12D3KooWMsPmAA65yHAHgbxgh7CPkEctJHZMeM3rAvoW8CZKxtpG`
  */
 export async function lookupMinerPeerId (minerId) {
-  const res = await rpc('Filecoin.StateMinerInfo', minerId, null)
-  return res.PeerId
+  try {
+    const res = await rpc('Filecoin.StateMinerInfo', minerId, null)
+    return res.PeerId
+  } catch (err) {
+    err.message = `Cannot lookup miner ${minerId}: ${err.message}`
+    throw err
+  }
 }
 
 /**
diff --git a/lib/spark.js b/lib/spark.js
index dc42c73..68f86c9 100644
--- a/lib/spark.js
+++ b/lib/spark.js
@@ -48,7 +48,23 @@ export default class Spark {
       stats.providerId = peerId
     } catch (err) {
       console.error(err)
-      this.#activity.onError()
+      // There are three common error cases:
+      //  1. We are offline
+      //  2. The JSON RPC provider is down
+      //  3. JSON RPC errors like when Miner ID is not a known actor
+      // There isn't much we can do in the first two cases. We can notify the user that we are not
+      // performing any jobs and wait until the problem is resolved.
+      // The third case should not happen unless we made a mistake, so we want to learn about it
+      if (err.name === 'FilecoinRpcError') {
+        // TODO: report the error to Sentry
+        console.error('The error printed above was not expected, please report it on GitHub:')
+        console.error('https://github.com/filecoin-station/spark/issues/new')
+      } else {
+        this.#activity.onError()
+      }
+      err.reported = true
+      // Abort the check, no measurement should be recorded
+      throw err
     }
 
     console.log(`Querying IPNI to find retrieval providers for ${retrieval.cid}`)
@@ -200,12 +216,7 @@ export default class Spark {
         await this.nextRetrieval()
         this.#activity.onHealthy()
       } catch (err) {
-        if (err.statusCode === 400 && err.serverMessage === 'OUTDATED CLIENT') {
-          this.#activity.onOutdatedClient()
-        } else {
-          this.#activity.onError()
-        }
-        console.error(err)
+        this.handleRunError(err)
       }
       const duration = Date.now() - started
       const baseDelay = APPROX_ROUND_LENGTH_IN_MS / this.#maxTasksPerNode
@@ -216,6 +227,17 @@ export default class Spark {
       }
     }
   }
+
+  handleRunError (err) {
+    if (err.reported) return
+
+    if (err.statusCode === 400 && err.serverMessage === 'OUTDATED CLIENT') {
+      this.#activity.onOutdatedClient()
+    } else {
+      this.#activity.onError()
+    }
+    console.error(err)
+  }
 }
 
 async function assertOkResponse (res, errorMsg) {
diff --git a/test/miner-lookup.test.js b/test/miner-lookup.test.js
index 27eefb9..722d32f 100644
--- a/test/miner-lookup.test.js
+++ b/test/miner-lookup.test.js
@@ -16,6 +16,6 @@ test('lookup peer id of a miner that does not exist', async () => {
       `Expected "lookupMinerPeerId()" to fail, but it resolved with "${result}" instead.`
     )
   } catch (err) {
-    assertMatch(err.toString(), /actor code is not miner/)
+    assertMatch(err.toString(), /\bf010\b.*\bactor code is not miner/)
   }
 })