From 33c5ad64d79bcf9f8b3a4ee2d523db869cce8748 Mon Sep 17 00:00:00 2001 From: Claudio Costa Date: Fri, 12 Jan 2024 16:59:09 -0600 Subject: [PATCH] Increase publish retry attempts (#61) --- build/pkgs_list | 12 ++-- cmd/recorder/recorder.go | 22 +------ cmd/recorder/upload.go | 30 +++++++++ cmd/recorder/upload_test.go | 117 ++++++++++++++++++++++++++++++++++++ 4 files changed, 156 insertions(+), 25 deletions(-) diff --git a/build/pkgs_list b/build/pkgs_list index 61056bb..37e827d 100644 --- a/build/pkgs_list +++ b/build/pkgs_list @@ -1,9 +1,9 @@ ca-certificates=20230311 -chromium=120.0.6099.71-1 -chromium-driver=120.0.6099.71-1 -chromium-sandbox=120.0.6099.71-1 -ffmpeg=7:6.1-5 +chromium=120.0.6099.216-1 +chromium-driver=120.0.6099.216-1 +chromium-sandbox=120.0.6099.216-1 +ffmpeg=7:6.1.1-1 fonts-recommended=1 -pulseaudio=16.1+dfsg1-2+b1 +pulseaudio=16.1+dfsg1-3 wget=1.21.4-1+b1 -xvfb=2:21.1.9-1 +xvfb=2:21.1.10-1 diff --git a/cmd/recorder/recorder.go b/cmd/recorder/recorder.go index 64f4ea4..49d2820 100644 --- a/cmd/recorder/recorder.go +++ b/cmd/recorder/recorder.go @@ -32,8 +32,6 @@ const ( displayID = 45 readyTimeout = 20 * time.Second stopTimeout = 10 * time.Second - maxUploadRetryAttempts = 5 - uploadRetryAttemptWaitTime = 5 * time.Second connCheckInterval = 1 * time.Second initCheckInterval = 1 * time.Second initCheckTimeout = 5 * time.Second @@ -426,25 +424,11 @@ func (rec *Recorder) Stop() error { return exitErr } - // TODO (MM-48546): implement better retry logic. - var attempt int - for { - err := rec.uploadRecording() - if err == nil { - slog.Info("recording uploaded successfully") - break - } - - if attempt == maxUploadRetryAttempts { - return fmt.Errorf("max retry attempts reached, exiting") - } - - attempt++ - slog.Info("failed to upload recording", slog.String("err", err.Error())) - slog.Info("retrying", slog.Duration("wait_time", uploadRetryAttemptWaitTime)) - time.Sleep(uploadRetryAttemptWaitTime) + if err := rec.publishRecording(); err != nil { + return fmt.Errorf("failed to publish recording: %w", err) } + slog.Debug("upload successful, removing file", slog.String("outpath", rec.outPath)) if err := os.Remove(rec.outPath); err != nil { slog.Error("failed to remove recording", slog.String("err", err.Error())) } diff --git a/cmd/recorder/upload.go b/cmd/recorder/upload.go index 466db0b..3752ebe 100644 --- a/cmd/recorder/upload.go +++ b/cmd/recorder/upload.go @@ -4,6 +4,7 @@ import ( "context" "encoding/json" "fmt" + "log/slog" "net/http" "os" "path/filepath" @@ -19,6 +20,35 @@ const ( httpUploadTimeout = 5 * time.Minute ) +var ( + maxUploadRetryAttempts = 20 + uploadRetryAttemptWaitTime = 5 * time.Second +) + +func (rec *Recorder) publishRecording() error { + var attempt int + for { + err := rec.uploadRecording() + if err == nil { + slog.Info("recording uploaded successfully") + break + } + + if attempt == maxUploadRetryAttempts { + return fmt.Errorf("max retry attempts reached, exiting") + } + + attempt++ + slog.Info("failed to upload recording", slog.String("err", err.Error())) + + waitTime := uploadRetryAttemptWaitTime * time.Duration(attempt) + slog.Info("retrying", slog.Duration("wait_time", waitTime)) + time.Sleep(waitTime) + } + + return nil +} + func (rec *Recorder) uploadRecording() error { file, err := os.Open(rec.outPath) if err != nil { diff --git a/cmd/recorder/upload_test.go b/cmd/recorder/upload_test.go index e3252fd..bd1233f 100644 --- a/cmd/recorder/upload_test.go +++ b/cmd/recorder/upload_test.go @@ -2,10 +2,12 @@ package main import ( "fmt" + "log/slog" "net/http" "net/http/httptest" "os" "testing" + "time" "github.com/mattermost/calls-recorder/cmd/recorder/config" @@ -135,3 +137,118 @@ func TestUploadRecording(t *testing.T) { require.NoError(t, err) }) } + +func TestPublishRecording(t *testing.T) { + logger := slog.New(slog.NewTextHandler(os.Stdout, &slog.HandlerOptions{ + AddSource: true, + Level: slog.LevelDebug, + })) + slog.SetDefault(logger) + + middlewares := []middleware{} + ts := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + for _, mw := range middlewares { + if mw(w, r) { + return + } + } + http.NotFound(w, r) + })) + defer ts.Close() + + cfg := config.RecorderConfig{ + SiteURL: ts.URL, + CallID: "8w8jorhr7j83uqr6y1st894hqe", + PostID: "udzdsg7dwidbzcidx5khrf8nee", + RecordingID: "67t5u6cmtfbb7jug739d43xa9e", + AuthToken: "qj75unbsef83ik9p7ueypb6iyw", + } + cfg.SetDefaults() + rec, err := NewRecorder(cfg) + require.NoError(t, err) + require.NotNil(t, rec) + + recFile, err := os.CreateTemp("", "recording.mp4") + require.NoError(t, err) + defer os.Remove(recFile.Name()) + + rec.outPath = recFile.Name() + + uploadRetryAttemptWaitTime = time.Second + + t.Run("success after multiple failed attempts", func(t *testing.T) { + var failures int + middlewares = []middleware{ + func(w http.ResponseWriter, r *http.Request) bool { + if r.URL.Path == "/plugins/com.mattermost.calls/bot/uploads" && r.Method == http.MethodPost { + fmt.Fprintln(w, `{"id": "uploadID"}`) + return true + } + + return false + }, + func(w http.ResponseWriter, r *http.Request) bool { + if r.URL.Path == "/plugins/com.mattermost.calls/bot/uploads/uploadID" && r.Method == http.MethodPost { + fmt.Fprintln(w, `{"id": "fileID"}`) + return true + } + + return false + }, + func(w http.ResponseWriter, r *http.Request) bool { + if r.URL.Path == "/plugins/com.mattermost.calls/bot/calls/8w8jorhr7j83uqr6y1st894hqe/recordings" && r.Method == http.MethodPost { + if failures < 5 { + w.WriteHeader(400) + fmt.Fprintln(w, `{"message": "server error"}`) + failures++ + } else { + w.WriteHeader(200) + } + return true + } + return false + }, + } + + err := rec.publishRecording() + require.NoError(t, err) + }) + + t.Run("failure after maximum attempts reached", func(t *testing.T) { + uploadRetryAttemptWaitTime = 10 * time.Millisecond + + middlewares[1] = func(w http.ResponseWriter, r *http.Request) bool { + if r.URL.Path == "/plugins/com.mattermost.calls/bot/uploads/uploadID" && r.Method == http.MethodPost { + w.WriteHeader(500) + fmt.Fprintln(w, `{"message": "server error"}`) + return true + } + + return false + } + + err := rec.publishRecording() + require.EqualError(t, err, "max retry attempts reached, exiting") + }) + + t.Run("success", func(t *testing.T) { + middlewares[1] = func(w http.ResponseWriter, r *http.Request) bool { + if r.URL.Path == "/plugins/com.mattermost.calls/bot/uploads/uploadID" && r.Method == http.MethodPost { + fmt.Fprintln(w, `{"id": "fileID"}`) + return true + } + return false + } + + middlewares[2] = func(w http.ResponseWriter, r *http.Request) bool { + if r.URL.Path == "/plugins/com.mattermost.calls/bot/calls/8w8jorhr7j83uqr6y1st894hqe/recordings" && r.Method == http.MethodPost { + w.WriteHeader(200) + return true + } + return false + } + + err := rec.publishRecording() + require.NoError(t, err) + }) +}