From ce367808ded084ef7b0b69438157b964a76d1d10 Mon Sep 17 00:00:00 2001 From: Thom Shutt Date: Wed, 7 Sep 2022 13:38:53 +0100 Subject: [PATCH 1/3] Add linter as Github step --- .github/workflows/golangci-lint.yml | 46 +++++++++++++++++++++++++++++ 1 file changed, 46 insertions(+) create mode 100644 .github/workflows/golangci-lint.yml diff --git a/.github/workflows/golangci-lint.yml b/.github/workflows/golangci-lint.yml new file mode 100644 index 000000000..388a1ce66 --- /dev/null +++ b/.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 From 959cfb2903057bca309749596a1790e41e16eafe Mon Sep 17 00:00:00 2001 From: Thom Shutt Date: Wed, 7 Sep 2022 13:40:24 +0100 Subject: [PATCH 2/3] Fix linting errors --- api/http.go | 2 +- clients/callback_client_test.go | 8 ++++---- errors/errors.go | 8 ++++---- handlers/ok.go | 5 ++++- handlers/transcode.go | 2 +- handlers/triggers.go | 31 ++++++++++++++++++++++++++----- handlers/upload.go | 4 +++- middleware/logging.go | 4 ++-- 8 files changed, 45 insertions(+), 19 deletions(-) diff --git a/api/http.go b/api/http.go index e317ea2a4..7c3acfe0a 100644 --- a/api/http.go +++ b/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, diff --git a/clients/callback_client_test.go b/clients/callback_client_test.go index 21c6207ab..15e104bfd 100644 --- a/clients/callback_client_test.go +++ b/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)) diff --git a/errors/errors.go b/errors/errors.go index 272cee842..e172a85e9 100644 --- a/errors/errors.go +++ b/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} } diff --git a/handlers/ok.go b/handlers/ok.go index 50a1229d4..8569b04fe 100644 --- a/handlers/ok.go +++ b/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) + } } } diff --git a/handlers/transcode.go b/handlers/transcode.go index 88476bb77..34d85bed7 100644 --- a/handlers/transcode.go +++ b/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 } } diff --git a/handlers/triggers.go b/handlers/triggers.go index 6b6603ae0..48a448fa1 100644 --- a/handlers/triggers.go +++ b/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 + } } diff --git a/handlers/upload.go b/handlers/upload.go index fe784c96a..60268d92f 100644 --- a/handlers/upload.go +++ b/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) + } } } diff --git a/middleware/logging.go b/middleware/logging.go index 35a6a97b6..8173ba253 100644 --- a/middleware/logging.go +++ b/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, From e7139c333fb5628cea803a8da3e017a8c626b782 Mon Sep 17 00:00:00 2001 From: Thom Shutt Date: Wed, 7 Sep 2022 13:43:44 +0100 Subject: [PATCH 3/3] Add instructions for installing linter --- Makefile | 6 +++++- README.md | 6 +++++- 2 files changed, 10 insertions(+), 2 deletions(-) diff --git a/Makefile b/Makefile index 49f5b1fc8..85cfd75a1 100644 --- a/Makefile +++ b/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 diff --git a/README.md b/README.md index 4e9b34ebf..f01cc1892 100644 --- a/README.md +++ b/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. \ No newline at end of file +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.