Add linting as CI step

.github/workflows/golangci-lint.yml

@@ -0,0 +1,46 @@
+name: golangci-lint
+on:
+  push:
+    tags:
+      - v*
+    branches:
+      - master
+      - main
+  pull_request:
+permissions:
+  contents: read
+  # Optional: allow read access to pull request. Use with `only-new-issues` option.
+  # pull-requests: read
+jobs:
+  golangci:
+    name: lint
+    runs-on: ubuntu-latest
+    steps:
+      - uses: actions/setup-go@v3
+        with:
+          go-version: 1.19
+      - uses: actions/checkout@v3
+      - name: golangci-lint
+        uses: golangci/golangci-lint-action@v3
+        with:
+          # Optional: version of golangci-lint to use in form of v1.2 or v1.2.3 or `latest` to use the latest version
+          version: latest
+
+          # Optional: working directory, useful for monorepos
+          # working-directory: somedir
+
+          # Optional: golangci-lint command line arguments.
+          # args: --issues-exit-code=0
+
+          # Optional: show only new issues if it's a pull request. The default value is `false`.
+          # only-new-issues: true
+
+          # Optional: if set to true then the all caching functionality will be complete disabled,
+          #           takes precedence over all other caching options.
+          # skip-cache: true
+
+          # Optional: if set to true then the action don't cache or restore ~/go/pkg.
+          # skip-pkg-cache: true
+
+          # Optional: if set to true then the action don't cache or restore ~/.cache/go-build.
+          # skip-build-cache: true

Makefile

@@ -3,7 +3,7 @@ GO_BUILD_DIR?=build/
 ldflags := -X 'github.com/livepeer/catalyst-api/config.Version=$(shell git rev-parse HEAD)'
 
 .PHONY: all
-all: build fmt test
+all: build fmt test lint
 
 .PHONY: build
 build:
@@ -13,6 +13,10 @@ build:
 fmt:
 	go fmt ./...
 
+.PHONY: lint
+lint:
+	golangci-lint run
+
 .PHONY: run
 run:
 	go run cmd/http-server.go

README.md

@@ -2,6 +2,10 @@
 
 An HTTP API to allow services (e.g Livepeer Studio) to interact with Catalyst.
 
+## Prerequisites
+
+The default Makefile goal and the CI process for this repo uses `golangci-lint` for linting. To install, follow the instructions at https://golangci-lint.run/usage/install/#local-installation
+
 ## Development
 
 To test the [Catalyst](http://github.com/livepeer/catalyst) integration, follow the instructions in that repo to run a local Catalyst instance and then:
@@ -18,4 +22,4 @@ By default, this runs on a different port (`4949`) to the Catalyst one (`7979`)
 curl 'http://localhost:4949/ok'
 ```
 
-If you see a response body of `OK` then things are working and you can begin using this instance to test the API's integration with Mist and the other parts of the Catalyst system.
+If you see a response body of `OK` then things are working and you can begin using this instance to test the API's integration with Mist and the other parts of the Catalyst system.

api/http.go

@@ -20,7 +20,7 @@ func ListenAndServe(apiPort, mistPort int) error {
 	listen := fmt.Sprintf("0.0.0.0:%d", apiPort)
 	router := NewCatalystAPIRouter(mc)
 
-	config.Logger.Log(
+	_ = config.Logger.Log(
 		"msg", "Starting Catalyst API",
 		"version", config.Version,
 		"host", listen,

clients/callback_client_test.go

@@ -2,7 +2,7 @@ package clients
 
 import (
 	"fmt"
-	"io/ioutil"
+	"io"
 	"net/http"
 	"net/http/httptest"
 	"testing"
@@ -21,7 +21,7 @@ func TestItRetriesOnFailedCallbacks(t *testing.T) {
 	// Set up a dummy server to receive the callbacks
 	svr := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
 		// Check that we got the callback we're expecting
-		body, err := ioutil.ReadAll(r.Body)
+		body, err := io.ReadAll(r.Body)
 		require.NoError(t, err)
 		require.JSONEq(t, `{"completion_ratio":1, "status":"success", "timestamp": 123456789}`, string(body))
 
@@ -53,7 +53,7 @@ func TestItEventuallyStopsRetrying(t *testing.T) {
 	// Set up a dummy server to receive the callbacks
 	svr := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
 		// Check that we got the callback we're expecting
-		body, err := ioutil.ReadAll(r.Body)
+		body, err := io.ReadAll(r.Body)
 		require.NoError(t, err)
 		require.JSONEq(t, `{"completion_ratio":1, "status":"success", "timestamp": 123456789}`, string(body))
 
@@ -80,7 +80,7 @@ func TestTranscodeStatusErrorNotifcation(t *testing.T) {
 	// Set up a dummy server to receive the callbacks
 	svr := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
 		// Check that we got the callback we're expecting
-		body, err := ioutil.ReadAll(r.Body)
+		body, err := io.ReadAll(r.Body)
 		require.NoError(t, err)
 		require.JSONEq(t, `{"completion_ratio": 0, "error": "something went wrong", "status":"error", "timestamp": 123456789}`, string(body))
 

errors/errors.go

@@ -2,10 +2,10 @@ package errors
 
 import (
 	"encoding/json"
-	"log"
 	"net/http"
 	"strings"
 
+	"github.com/livepeer/catalyst-api/config"
 	"github.com/xeipuuv/gojsonschema"
 )
 
@@ -17,9 +17,9 @@ type apiError struct {
 
 func writeHttpError(w http.ResponseWriter, msg string, status int, err error) apiError {
 	w.WriteHeader(status)
-	json.NewEncoder(w).Encode(map[string]string{"error": msg})
-	if err != nil {
-		log.Println(msg, err)
+
+	if err := json.NewEncoder(w).Encode(map[string]string{"error": msg}); err != nil {
+		_ = config.Logger.Log("msg", "error writing HTTP error", "http_error_msg", msg, "error", err)
 	}
 	return apiError{msg, status, err}
 }

handlers/ok.go

@@ -5,10 +5,13 @@ import (
 	"net/http"
 
 	"github.com/julienschmidt/httprouter"
+	"github.com/livepeer/catalyst-api/config"
 )
 
 func (d *CatalystAPIHandlersCollection) Ok() httprouter.Handle {
 	return func(w http.ResponseWriter, req *http.Request, _ httprouter.Params) {
-		io.WriteString(w, "OK")
+		if _, err := io.WriteString(w, "OK"); err != nil {
+			_ = config.Logger.Log("error", "Failed to write HTTP response for "+req.URL.RawPath)
+		}
 	}
 }

handlers/transcode.go

@@ -74,6 +74,6 @@ func (d *CatalystAPIHandlersCollection) TranscodeSegment() httprouter.Handle {
 		if err := CallbackClient.SendTranscodeStatusError(transcodeRequest.CallbackUrl, "NYI - not yet implemented"); err != nil {
 			errors.WriteHTTPInternalServerError(w, "error send transcode error", err)
 		}
-		io.WriteString(w, "OK") // TODO later
+		_, _ = io.WriteString(w, "OK") // TODO later
 	}
 }

handlers/triggers.go

@@ -9,6 +9,7 @@ import (
 
 	"github.com/julienschmidt/httprouter"
 	"github.com/livepeer/catalyst-api/clients"
+	"github.com/livepeer/catalyst-api/config"
 	"github.com/livepeer/catalyst-api/errors"
 )
 
@@ -60,15 +61,31 @@ func (d *MistCallbackHandlersCollection) Trigger() httprouter.Handle {
 // in place that we'll need
 func stubTranscodingCallbacksForStudio(callbackURL string, callbackClient clients.CallbackClient) {
 	time.Sleep(5 * time.Second)
-	callbackClient.SendTranscodeStatus(callbackURL, clients.TranscodeStatusTranscoding, 0.3)
+	if err := callbackClient.SendTranscodeStatus(callbackURL, clients.TranscodeStatusTranscoding, 0.3); err != nil {
+		_ = config.Logger.Log("msg", "Error in stubTranscodingCallbacksForStudio", "err", err)
+		return
+	}
+
 	time.Sleep(5 * time.Second)
-	callbackClient.SendTranscodeStatus(callbackURL, clients.TranscodeStatusTranscoding, 0.6)
+	if err := callbackClient.SendTranscodeStatus(callbackURL, clients.TranscodeStatusTranscoding, 0.6); err != nil {
+		_ = config.Logger.Log("msg", "Error in stubTranscodingCallbacksForStudio", "err", err)
+		return
+	}
+
 	time.Sleep(5 * time.Second)
-	callbackClient.SendTranscodeStatus(callbackURL, clients.TranscodeStatusTranscoding, 0.9)
+	if err := callbackClient.SendTranscodeStatus(callbackURL, clients.TranscodeStatusTranscoding, 0.9); err != nil {
+		_ = config.Logger.Log("msg", "Error in stubTranscodingCallbacksForStudio", "err", err)
+		return
+	}
+
 	time.Sleep(5 * time.Second)
-	callbackClient.SendTranscodeStatus(callbackURL, clients.TranscodeStatusTranscoding, 1)
+	if err := callbackClient.SendTranscodeStatus(callbackURL, clients.TranscodeStatusTranscoding, 1); err != nil {
+		_ = config.Logger.Log("msg", "Error in stubTranscodingCallbacksForStudio", "err", err)
+		return
+	}
+
 	time.Sleep(5 * time.Second)
-	callbackClient.SendTranscodeStatusCompleted(
+	err := callbackClient.SendTranscodeStatusCompleted(
 		callbackURL,
 		clients.InputVideo{
 			Format:   "mp4",
@@ -112,4 +129,8 @@ func stubTranscodingCallbacksForStudio(callbackURL string, callbackClient client
 			},
 		},
 	)
+	if err != nil {
+		_ = config.Logger.Log("msg", "Error sending Transcode Completed in stubTranscodingCallbacksForStudio", "err", err)
+		return
+	}
 }

handlers/upload.go

@@ -97,7 +97,9 @@ func (d *CatalystAPIHandlersCollection) UploadVOD() httprouter.Handle {
 			errors.WriteHTTPInternalServerError(w, "Cannot send transcode status", err)
 		}
 
-		io.WriteString(w, fmt.Sprint(len(uploadVODRequest.OutputLocations)))
+		if _, err := io.WriteString(w, fmt.Sprint(len(uploadVODRequest.OutputLocations))); err != nil {
+			errors.WriteHTTPInternalServerError(w, "Cannot write output locations", err)
+		}
 	}
 }
 

middleware/logging.go

@@ -39,12 +39,12 @@ func LogRequest() func(httprouter.Handle) httprouter.Handle {
 			defer func() {
 				if err := recover(); err != nil {
 					errors.WriteHTTPInternalServerError(wrapped, "Internal Server Error", nil)
-					config.Logger.Log("err", err, "trace", debug.Stack())
+					_ = config.Logger.Log("err", err, "trace", debug.Stack())
 				}
 			}()
 
 			next(wrapped, r, ps)
-			config.Logger.Log(
+			_ = config.Logger.Log(
 				"remote", r.RemoteAddr,
 				"proto", r.Proto,
 				"method", r.Method,