Increase publish retry attempts (#61)

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

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()))
 	}

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 {

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)
+	})
+}