fix(filters): external webhook remove jitter (#1254)

* fix(filters): external webhook remove jitter

* fix: migrations

---------

Co-authored-by: ze0s <ze0s@riseup.net>

internal/database/filter.go

@@ -250,7 +250,6 @@ func (r *FilterRepo) FindByID(ctx context.Context, filterID int) (*domain.Filter
 			"fe.webhook_retry_status",
 			"fe.webhook_retry_attempts",
 			"fe.webhook_retry_delay_seconds",
-			"fe.webhook_retry_max_jitter_seconds",
 		).
 		From("filter f").
 		LeftJoin("filter_external fe ON f.id = fe.filter_id").
@@ -420,7 +419,6 @@ func (r *FilterRepo) FindByID(ctx context.Context, filterID int) (*domain.Filter
 				WebhookRetryStatus:       extWebhookRetryStatus.String,
 				WebhookRetryAttempts:     int(extWebhookRetryAttempts.Int32),
 				WebhookRetryDelaySeconds: int(extWebhookDelaySeconds.Int32),
-				WebhookRetryMaxJitterSeconds: int(extWebhookRetryJitterSeconds.Int32),
 			}
 			externalMap[external.ID] = external
 		}
@@ -517,7 +515,6 @@ func (r *FilterRepo) findByIndexerIdentifier(ctx context.Context, indexer string
 			"fe.webhook_retry_status",
 			"fe.webhook_retry_attempts",
 			"fe.webhook_retry_delay_seconds",
-			"fe.webhook_retry_max_jitter_seconds",
 			"fe.filter_id",
 		).
 		From("filter f").
@@ -694,7 +691,6 @@ func (r *FilterRepo) findByIndexerIdentifier(ctx context.Context, indexer string
 				WebhookRetryStatus:       extWebhookRetryStatus.String,
 				WebhookRetryAttempts:     int(extWebhookRetryAttempts.Int32),
 				WebhookRetryDelaySeconds: int(extWebhookDelaySeconds.Int32),
-				WebhookRetryMaxJitterSeconds: int(extWebhookRetryJitterSeconds.Int32),
 				FilterId:                 int(extFilterId.Int32),
 			}
 			externalMap[external.FilterId] = append(externalMap[external.FilterId], external)
@@ -736,7 +732,6 @@ func (r *FilterRepo) FindExternalFiltersByID(ctx context.Context, filterId int)
 			"fe.webhook_retry_status",
 			"fe.webhook_retry_attempts",
 			"fe.webhook_retry_delay_seconds",
-			"fe.webhook_retry_max_jitter_seconds",
 		).
 		From("filter_external fe").
 		Where(sq.Eq{"fe.filter_id": filterId})
@@ -797,7 +792,6 @@ func (r *FilterRepo) FindExternalFiltersByID(ctx context.Context, filterId int)
 		external.WebhookRetryStatus = extWebhookRetryStatus.String
 		external.WebhookRetryAttempts = int(extWebhookRetryAttempts.Int32)
 		external.WebhookRetryDelaySeconds = int(extWebhookDelaySeconds.Int32)
-		external.WebhookRetryMaxJitterSeconds = int(extWebhookRetryJitterSeconds.Int32)
 
 		externalFilters = append(externalFilters, external)
 	}
@@ -1227,9 +1221,6 @@ func (r *FilterRepo) UpdatePartial(ctx context.Context, filter domain.FilterUpda
 	if filter.ExternalWebhookRetryDelaySeconds != nil {
 		q = q.Set("external_webhook_retry_delay_seconds", filter.ExternalWebhookRetryDelaySeconds)
 	}
-	if filter.ExternalWebhookRetryMaxJitterSeconds != nil {
-		q = q.Set("external_webhook_retry_max_jitter_seconds", filter.ExternalWebhookRetryMaxJitterSeconds)
-	}
 
 	q = q.Where(sq.Eq{"id": filter.ID})
 
@@ -1513,7 +1504,6 @@ func (r *FilterRepo) StoreFilterExternal(ctx context.Context, filterID int, exte
 			"webhook_retry_status",
 			"webhook_retry_attempts",
 			"webhook_retry_delay_seconds",
-			"webhook_retry_max_jitter_seconds",
 			"filter_id",
 		)
 
@@ -1534,7 +1524,6 @@ func (r *FilterRepo) StoreFilterExternal(ctx context.Context, filterID int, exte
 			toNullString(external.WebhookRetryStatus),
 			toNullInt32(int32(external.WebhookRetryAttempts)),
 			toNullInt32(int32(external.WebhookRetryDelaySeconds)),
-			toNullInt32(int32(external.WebhookRetryMaxJitterSeconds)),
 			filterID,
 		)
 	}

internal/database/postgres_migrate.go

@@ -149,7 +149,6 @@ CREATE TABLE filter_external
     webhook_retry_status                TEXT,
     webhook_retry_attempts              INTEGER,
     webhook_retry_delay_seconds         INTEGER,
-    webhook_retry_max_jitter_seconds    INTEGER,
     filter_id                           INTEGER NOT NULL,
     FOREIGN KEY (filter_id)             REFERENCES filter(id) ON DELETE CASCADE
 );
@@ -825,5 +824,8 @@ ALTER TABLE filter_external
 
 ALTER TABLE filter_external
     RENAME COLUMN external_webhook_retry_max_jitter_seconds TO webhook_retry_max_jitter_seconds;
+`,
+	`ALTER TABLE filter_external
+	DROP COLUMN IF EXISTS external_webhook_retry_max_jitter_seconds;
 `,
 }

internal/database/sqlite_migrate.go

@@ -149,7 +149,6 @@ CREATE TABLE filter_external
     webhook_retry_status                TEXT,
     webhook_retry_attempts              INTEGER,
     webhook_retry_delay_seconds         INTEGER,
-    webhook_retry_max_jitter_seconds    INTEGER,
     filter_id                           INTEGER NOT NULL,
     FOREIGN KEY (filter_id)             REFERENCES filter(id) ON DELETE CASCADE
 );
@@ -1418,5 +1417,8 @@ DROP TABLE filter_external;
 
 ALTER TABLE filter_external_dg_tmp
     RENAME TO filter_external;
+`,
+	`ALTER TABLE filter_external
+	DROP COLUMN external_webhook_retry_max_jitter_seconds;
 `,
 }

internal/domain/filter.go

@@ -154,7 +154,6 @@ type FilterExternal struct {
 	WebhookRetryStatus       string             `json:"webhook_retry_status,omitempty"`
 	WebhookRetryAttempts     int                `json:"webhook_retry_attempts,omitempty"`
 	WebhookRetryDelaySeconds int                `json:"webhook_retry_delay_seconds,omitempty"`
-	WebhookRetryMaxJitterSeconds int                `json:"webhook_retry_max_jitter_seconds,omitempty"`
 	FilterId                 int                `json:"-"`
 }
 
@@ -239,7 +238,6 @@ type FilterUpdate struct {
 	ExternalWebhookRetryStatus       *string                 `json:"external_webhook_retry_status,omitempty"`
 	ExternalWebhookRetryAttempts     *int                    `json:"external_webhook_retry_attempts,omitempty"`
 	ExternalWebhookRetryDelaySeconds *int                    `json:"external_webhook_retry_delay_seconds,omitempty"`
-	ExternalWebhookRetryMaxJitterSeconds *int                    `json:"external_webhook_retry_max_jitter_seconds,omitempty"`
 	Actions                          []*Action               `json:"actions,omitempty"`
 	External                         []FilterExternal        `json:"external,omitempty"`
 	Indexers                         []Indexer               `json:"indexers,omitempty"`

internal/filter/service.go

@@ -4,7 +4,6 @@
 package filter
 
 import (
-	"bufio"
 	"bytes"
 	"context"
 	"crypto/tls"
@@ -704,21 +703,11 @@ func (s *service) webhook(ctx context.Context, external domain.FilterExternal, r
 		method = external.WebhookMethod
 	}
 
-	var req *http.Request
+	req, err := http.NewRequestWithContext(ctx, method, external.WebhookHost, nil)
-	if external.WebhookData != "" && dataArgs != "" {
-		req, err = http.NewRequestWithContext(ctx, method, external.WebhookHost, bytes.NewBufferString(dataArgs))
 	if err != nil {
 		return 0, errors.Wrap(err, "could not build request for webhook")
 	}
 
-		defer req.Body.Close()
-	} else {
-		req, err = http.NewRequestWithContext(ctx, method, external.WebhookHost, nil)
-		if err != nil {
-			return 0, errors.Wrap(err, "could not build request for webhook")
-		}
-	}
-
 	req.Header.Set("Content-Type", "application/json")
 	req.Header.Set("User-Agent", "autobrr")
 
@@ -739,30 +728,29 @@ func (s *service) webhook(ctx context.Context, external domain.FilterExternal, r
 
 	var opts []retry.Option
 
+	opts = append(opts, retry.DelayType(retry.FixedDelay))
+	opts = append(opts, retry.LastErrorOnly(true))
+
 	if external.WebhookRetryAttempts > 0 {
-		option := retry.Attempts(uint(external.WebhookRetryAttempts))
+		opts = append(opts, retry.Attempts(uint(external.WebhookRetryAttempts)))
-		opts = append(opts, option)
 	}
 	if external.WebhookRetryDelaySeconds > 0 {
-		option := retry.Delay(time.Duration(external.WebhookRetryDelaySeconds) * time.Second)
+		opts = append(opts, retry.Delay(time.Duration(external.WebhookRetryDelaySeconds)*time.Second))
-		opts = append(opts, option)
 	}
-	if external.WebhookRetryMaxJitterSeconds > 0 {
-		option := retry.MaxJitter(time.Duration(external.WebhookRetryMaxJitterSeconds) * time.Second)
-		opts = append(opts, option)
-	}
-
-	start := time.Now()
 
 	var retryStatusCodes []string
 	if external.WebhookRetryStatus != "" {
 		retryStatusCodes = strings.Split(strings.ReplaceAll(external.WebhookRetryStatus, " ", ""), ",")
 	}
 
+	start := time.Now()
+
 	statusCode, err := retry.DoWithData(
 		func() (int, error) {
 			clonereq := req.Clone(ctx)
-			clonereq.Body = io.NopCloser(bufio.NewReader(req.Body))
+			if external.WebhookData != "" && dataArgs != "" {
+				clonereq.Body = io.NopCloser(bytes.NewBufferString(dataArgs))
+			}
 			res, err := client.Do(clonereq)
 			if err != nil {
 				return 0, errors.Wrap(err, "could not make request for webhook")
@@ -770,6 +758,8 @@ func (s *service) webhook(ctx context.Context, external domain.FilterExternal, r
 
 			defer res.Body.Close()
 
+			s.log.Debug().Msgf("filter external webhook response status: %d", res.StatusCode)
+
 			body, err := io.ReadAll(res.Body)
 			if err != nil {
 				return res.StatusCode, errors.Wrap(err, "could not read request body")
@@ -780,7 +770,7 @@ func (s *service) webhook(ctx context.Context, external domain.FilterExternal, r
 			}
 
 			if utils.StrSliceContains(retryStatusCodes, strconv.Itoa(res.StatusCode)) {
-				return 0, errors.New("retrying webhook request, got status code: %d", res.StatusCode)
+				return 0, errors.New("webhook got unwanted status code: %d", res.StatusCode)
 			}
 
 			return res.StatusCode, nil

web/src/screens/filters/Details.tsx

@@ -221,7 +221,6 @@ const externalFilterSchema = z.object({
   webhook_retry_status: z.string().optional(),
   webhook_retry_attempts: z.number().optional(),
   webhook_retry_delay_seconds: z.number().optional(),
-  webhook_retry_max_jitter_seconds: z.number().optional(),
 });
 
 const indexerSchema = z.object({
@@ -1344,4 +1343,3 @@ export function CollapsableSection({ title, subtitle, children, defaultOpen }: C
     </div>
   );
 }
-

web/src/screens/filters/External.tsx

@@ -332,11 +332,6 @@ const TypeForm = ({ external, idx }: TypeFormProps) => {
           label="Retry delay in seconds"
           placeholder="1"
         />
-        <NumberField
-          name={`external.${idx}.webhook_retry_max_jitter_seconds`}
-          label="Max jitter in seconds"
-          placeholder="1"
-        />
       </div>
     );
 

web/src/screens/filters/List.tsx

@@ -295,7 +295,6 @@ const FilterItemDropdown = ({ filter, onToggle }: FilterItemDropdownProps) => {
         external_webhook_retry_status: any;
         external_webhook_retry_attempts: any;
         external_webhook_retry_delay_seconds: any;
-        external_webhook_retry_max_jitter_seconds: any;
       };
 
       const completeFilter = await APIClient.filters.getByID(filter.id) as Partial<CompleteFilterType>;
@@ -320,7 +319,6 @@ const FilterItemDropdown = ({ filter, onToggle }: FilterItemDropdownProps) => {
       delete completeFilter.external_webhook_retry_status;
       delete completeFilter.external_webhook_retry_attempts;
       delete completeFilter.external_webhook_retry_delay_seconds;
-      delete completeFilter.external_webhook_retry_max_jitter_seconds;
 
       // Remove properties with default values from the exported filter to minimize the size of the JSON string
       ["enabled", "priority", "smart_episode", "resolutions", "sources", "codecs", "containers", "tags_match_logic", "except_tags_match_logic"].forEach((key) => {

web/src/types/Filter.d.ts

@@ -133,6 +133,5 @@ interface ExternalFilter {
   webhook_retry_status?: string,
   webhook_retry_attempts?: number;
   webhook_retry_delay_seconds?: number;
-  webhook_retry_max_jitter_seconds?: number;
   filter_id?: number;
 }