feat(core): support skip health check option

.github/workflows/kgw-test-reuse.yaml

@@ -109,29 +109,35 @@ jobs:
         # if kgw on non-release branches, we want to use go workspace, so that kgw
         # always uses the latest version of kwil-db/core
         run: |
+          echo "KWK_DIR=$(pwd)/.." >> $GITHUB_ENV
           kdbDir=$(pwd)
+          echo "KDB_DIR=$(pwd)" >> $GITHUB_ENV
+          echo "current dir: " $kdbDir
+          cd ..
+          rm -rf ./kgw
           git config --global url."https://${GH_ACCESS_TOKEN}:[email protected]/kwilteam/".insteadOf "https://github.com/kwilteam/"
-          rm -rf /tmp/kgw
-          git clone -b ${{ inputs.kgw-ref }}  https://github.com/kwilteam/kgw.git /tmp/kgw
+          git clone -b ${{ inputs.kgw-ref }}  https://github.com/kwilteam/kgw.git ./kgw
           rm -rf ~/.gitconfig
-          cd /tmp/kgw
+          cd ./kgw
+          echo "KGW_DIR=$(pwd)" >> $GITHUB_ENV
           if [[ ${{ inputs.kgw-ref }} == release-* ]]; then
             go mod vendor
           else
             # non release branch, use go workspace to always use the latest version of kwil-db/core
-            go work init . $kdbDir/core
+            test -f go.work || go work init . ../kwil-db/core
             go work vendor
           fi
           cd -
+          cd $kdbDir
 
       - name: Build kgw image
         id: docker_build_kgw
         uses: docker/build-push-action@v5
         with:
-          context: /tmp/kgw
+          context: ${{ env.KGW_DIR }}/..
           load: true
           builder: ${{ steps.buildx.outputs.name }}
-          file: /tmp/kgw/Dockerfile
+          file: ${{ env.KGW_DIR }}/Dockerfile.workspace
           push: false
           tags: kgw:latest
           cache-from: type=local,src=/tmp/.buildx-cache-kgw

core/client/client.go

@@ -37,6 +37,9 @@ type Client struct {
 	// This is only effective when chainID is set.
 	skipVerifyChainID bool
 
+	// skipHealthcheck will skip check remote nodes' health status.
+	skipHealthcheck bool
+
 	noWarnings bool // silence warning logs
 
 	authCallRPC bool
@@ -107,33 +110,60 @@ func WrapClient(ctx context.Context, client user.TxSvcClient, options *clientTyp
 		chainID:           clientOptions.ChainID,
 		noWarnings:        clientOptions.Silence,
 		skipVerifyChainID: clientOptions.SkipVerifyChainID,
+		skipHealthcheck:   clientOptions.SkipHealthcheck,
 	}
 
-	health, err := c.Health(ctx)
-	if err != nil {
-		return nil, fmt.Errorf("failed to retrieve the node's health: %w", err)
-	}
+	var remoteChainID string
 
-	if health.Healthy {
-		c.logger.Warnf("node reports that it is not healthy: %v", health)
-	}
+	if c.skipHealthcheck {
+		health, err := c.Health(ctx)
+		// NOTE: we ignore all errors from c.Health call since we ignore health check
+		if err == nil {
+			// this is v09 API, we just take the result.
+			c.authCallRPC = health.Mode == types.ModePrivate
+			remoteChainID = health.ChainID
 
-	c.authCallRPC = health.Mode == types.ModePrivate
+			// NOTE: since original health check only log, why not ?
+			if health.Healthy {
+				c.logger.Warnf("node reports that it is not healthy: %v", health)
+			}
+		} else {
+			// fall back to v08 API
+			chainInfo, err := c.ChainInfo(ctx)
+			if err != nil {
+				return nil, fmt.Errorf("failed to retrieve the node's chain info: %w", err)
+			}
+
+			remoteChainID = chainInfo.ChainID
+		}
+	} else {
+		health, err := c.Health(ctx)
+		if err != nil {
+			return nil, fmt.Errorf("failed to retrieve the node's health: %w", err)
+		}
+
+		if health.Healthy {
+			c.logger.Warnf("node reports that it is not healthy: %v", health)
+		}
+
+		c.authCallRPC = health.Mode == types.ModePrivate
+		remoteChainID = health.ChainID
+	}
 
 	if c.chainID == "" { // always use chain ID from remote host
 		if !c.noWarnings {
 			c.logger.Warn("chain ID not set, trusting chain ID from remote host!",
-				zap.String("chainID", health.ChainID))
+				zap.String("chainID", remoteChainID))
 		}
 
-		c.chainID = health.ChainID
+		c.chainID = remoteChainID
 	} else {
 		if c.skipVerifyChainID {
 			if !c.noWarnings {
 				c.logger.Warn("chain ID is set, skip check against remote chain ID", zap.String("chainID", c.chainID))
 			}
-		} else if health.ChainID != c.chainID {
-			return nil, fmt.Errorf("remote host chain ID %q != client configured %q", health.ChainID, c.chainID)
+		} else if remoteChainID != c.chainID {
+			return nil, fmt.Errorf("remote host chain ID %q != client configured %q", remoteChainID, c.chainID)
 		}
 	}
 

core/rpc/client/error.go

@@ -11,6 +11,7 @@ var (
 	// It is the equivalent of http status code 401
 	ErrUnauthorized   = errors.New("unauthorized")
 	ErrNotFound       = errors.New("not found")
+	ErrMethodNotFound = errors.New("method not found")
 	ErrInvalidRequest = errors.New("invalid request")
 	ErrNotAllowed     = errors.New("not allowed")
 )

core/rpc/client/jsonrpc.go

@@ -208,7 +208,7 @@ func (cl *JSONRPCClient) CallMethod(ctx context.Context, method string, cmd, res
 // clientError joins a jsonrpc.Error with a client.RPCError and any appropriate
 // named error kind like ErrNotFound, ErrUnauthorized, etc. based on the code.
 func clientError(jsonRPCErr *jsonrpc.Error) error {
-	rpcErr := &RPCError{
+	rpcErr := &RPCError{ // @Jon, should we change this to RPCError instead of *RPCError ?
 		Msg:  jsonRPCErr.Message,
 		Code: int32(jsonRPCErr.Code),
 	}
@@ -218,9 +218,7 @@ func clientError(jsonRPCErr *jsonrpc.Error) error {
 	case jsonrpc.ErrorEngineDatasetNotFound, jsonrpc.ErrorTxNotFound, jsonrpc.ErrorValidatorNotFound:
 		return errors.Join(ErrNotFound, err)
 	case jsonrpc.ErrorUnknownMethod:
-		// TODO: change to client.ErrMethodNotFound. This should be different
-		// from other "not found" conditions
-		return errors.Join(ErrNotFound, err)
+		return errors.Join(ErrMethodNotFound, err)
 	// case jsonrpc.ErrorUnauthorized: // not yet used on server
 	// 	return errors.Join(client.ErrUnauthorized, err)
 	// case jsonrpc.ErrorInvalidSignature: // or leave this to core/client.Client to detect and report

core/types/client/opts.go

@@ -28,6 +28,9 @@ type Options struct {
 	// This option is only effective when ChainID is set.
 	SkipVerifyChainID bool
 
+	// SkipHealthcheck will skip check remote nodes' health status.
+	SkipHealthcheck bool
+
 	// Silence silences warnings logged from the client.
 	Silence bool
 
@@ -59,6 +62,8 @@ func (c *Options) Apply(opts *Options) {
 
 	c.SkipVerifyChainID = opts.SkipVerifyChainID
 
+	c.SkipHealthcheck = opts.SkipHealthcheck
+
 	c.Silence = opts.Silence
 }