From 3dfe83042e09d4989c7fb4119504c2bc91999159 Mon Sep 17 00:00:00 2001
From: Patrick Schork <patrick@eigenlabs.org>
Date: Sat, 11 May 2024 20:59:10 -0700
Subject: [PATCH] Refactor error handling to ensure that .env misconfigurations
 do not go unnoticed. - If NODE_DATAAPI_URL is not defined, we will continue
 to log error every interval. - If the constructed checkUrl is invalid, we
 will continue to log error every interval

---
 node/Makefile |  4 ++--
 node/node.go  | 25 +++++++++++++------------
 2 files changed, 15 insertions(+), 14 deletions(-)

diff --git a/node/Makefile b/node/Makefile
index 0bd4580866..9f2c0633e0 100644
--- a/node/Makefile
+++ b/node/Makefile
@@ -1,7 +1,7 @@
 GITCOMMIT := $(shell git rev-parse --short HEAD)
 GITDATE := $(shell git log -1 --format=%cd --date=unix)
 
-# GitVersion provides the semantic versioning for the project
+# GitVersion provides the semantic versioning for the project. If docker is not installed, semver fallsback to 0.0.0
 SEMVER := $(shell docker run --rm --volume "${PWD}/../:/repo" gittools/gitversion:5.12.0 /repo -output json -showvariable SemVer)
 ifeq ($(SEMVER), )
 SEMVER = "0.0.0"
@@ -19,7 +19,7 @@ docker: docker-node docker-plugin
 docker-node:
 	cd ../ && docker build --build-arg SEMVER=${SEMVER} --build-arg GITCOMMIT=${GITCOMMIT} --build-arg GITDATE=${GITDATE} . -t opr-node:${SEMVER} -t opr-node:latest -f node/cmd/Dockerfile
 
-docker-nodeplugin:
+docker-plugin:
 	cd ../ && docker build --build-arg SEMVER=${SEMVER} --build-arg GITCOMMIT=${GITCOMMIT} --build-arg GITDATE=${GITDATE} . -t opr-nodeplugin:${SEMVER} -t opr-nodeplugin:latest -f node/plugin/cmd/Dockerfile
 
 semver:
diff --git a/node/node.go b/node/node.go
index da70973f2c..95e0cb4e22 100644
--- a/node/node.go
+++ b/node/node.go
@@ -473,16 +473,6 @@ func (n *Node) checkNodeReachability() {
 		n.Logger.Warn("Node reachability checks disabled!!! ReachabilityPollIntervalSec set to 0")
 		return
 	}
-	if n.Config.DataApiUrl == "" {
-		n.Logger.Error("Node reachability checks disabled!!! DataAPI URL is not configured")
-		return
-	}
-
-	checkUrl, err := url.Parse(fmt.Sprintf("%s/api/v1/operators-info/port-check?operator_id=%s", n.Config.DataApiUrl, n.Config.ID.Hex()))
-	if err != nil {
-		n.Logger.Error("Node reachability checks disabled!!! Failed to parse reachability check url", err)
-		return
-	}
 
 	n.Logger.Info("Start nodeReachabilityCheck goroutine in background to check the reachability of the operator node")
 	ticker := time.NewTicker(time.Duration(n.Config.ReachabilityPollIntervalSec) * time.Second)
@@ -491,17 +481,28 @@ func (n *Node) checkNodeReachability() {
 	for {
 		<-ticker.C
 
+		if n.Config.DataApiUrl == "" {
+			n.Logger.Error("Unable to perform reachability check - NODE_DATAAPI_URL is not defined in .env")
+			continue
+		}
+
+		checkUrl, err := url.Parse(fmt.Sprintf("%s/api/v1/operators-info/port-check?operator_id=%s", n.Config.DataApiUrl, n.Config.ID.Hex()))
+		if err != nil {
+			n.Logger.Error("Reachability check failed - invalid check url", err, "checkUrl", checkUrl.String())
+			return
+		}
+
 		n.Logger.Info("Calling reachability check", "url", checkUrl.String())
 
 		resp, err := http.Get(checkUrl.String())
 		if err != nil {
-			n.Logger.Error("Reachability check failed", err)
+			n.Logger.Error("Reachability check request failed", err)
 			continue
 		} else if resp.StatusCode == 404 {
 			n.Logger.Error("Reachability check failed - operator id not found", "status", resp.StatusCode, "operator_id", n.Config.ID.Hex())
 			continue
 		} else if resp.StatusCode != 200 {
-			n.Logger.Error("Reachability check failed", "status", resp.StatusCode)
+			n.Logger.Error("Reachability check request failed", "status", resp.StatusCode)
 			continue
 		}