Browse Source

Fix: relabel tactic as FeedbackUploadMaxAttempts

- The label FeedbackUploadMaxRetries was incorrect
  because functionally it determined the number of
  upload attempts, not the number of subsequent
  retries after an initial upload attempt failed
mirokuratczyk 5 years ago
parent
commit
069383a34d
2 changed files with 10 additions and 9 deletions
  1. 2 2
      psiphon/common/parameters/clientParameters.go
  2. 8 7
      psiphon/feedback.go

+ 2 - 2
psiphon/common/parameters/clientParameters.go

@@ -245,7 +245,7 @@ const (
 	FeedbackUploadURLs                               = "FeedbackUploadURLs"
 	FeedbackEncryptionPublicKey                      = "FeedbackEncryptionPublicKey"
 	FeedbackTacticsWaitPeriod                        = "FeedbackTacticsWaitPeriod"
-	FeedbackUploadMaxRetries                         = "FeedbackUploadMaxRetries"
+	FeedbackUploadMaxAttempts                        = "FeedbackUploadMaxAttempts"
 	FeedbackUploadRetryMinDelaySeconds               = "FeedbackUploadRetryMinDelaySeconds"
 	FeedbackUploadRetryMaxDelaySeconds               = "FeedbackUploadRetryMaxDelaySeconds"
 	FeedbackUploadTimeoutSeconds                     = "FeedbackUploadTimeoutSeconds"
@@ -513,7 +513,7 @@ var defaultClientParameters = map[string]struct {
 	FeedbackUploadURLs:                 {value: TransferURLs{}},
 	FeedbackEncryptionPublicKey:        {value: ""},
 	FeedbackTacticsWaitPeriod:          {value: 5 * time.Second, minimum: 0 * time.Second, flags: useNetworkLatencyMultiplier},
-	FeedbackUploadMaxRetries:           {value: 5, minimum: 0},
+	FeedbackUploadMaxAttempts:          {value: 5, minimum: 0},
 	FeedbackUploadRetryMinDelaySeconds: {value: 1 * time.Minute, minimum: time.Duration(0), flags: useNetworkLatencyMultiplier},
 	FeedbackUploadRetryMaxDelaySeconds: {value: 5 * time.Minute, minimum: 1 * time.Second, flags: useNetworkLatencyMultiplier},
 	FeedbackUploadTimeoutSeconds:       {value: 30 * time.Second, minimum: 0 * time.Second, flags: useNetworkLatencyMultiplier},

+ 8 - 7
psiphon/feedback.go

@@ -124,7 +124,7 @@ func SendFeedback(ctx context.Context, configJson, diagnosticsJson, uploadPath s
 	feedbackUploadMinRetryDelay := p.Duration(parameters.FeedbackUploadRetryMinDelaySeconds)
 	feedbackUploadMaxRetryDelay := p.Duration(parameters.FeedbackUploadRetryMaxDelaySeconds)
 	feedbackUploadTimeout := p.Duration(parameters.FeedbackUploadTimeoutSeconds)
-	feedbackUploadMaxRetries := p.Int(parameters.FeedbackUploadMaxRetries)
+	feedbackUploadMaxAttempts := p.Int(parameters.FeedbackUploadMaxAttempts)
 	transferURLs := p.TransferURLs(parameters.FeedbackUploadURLs)
 	p.Close()
 
@@ -139,7 +139,7 @@ func SendFeedback(ctx context.Context, configJson, diagnosticsJson, uploadPath s
 
 	uploadId := prng.HexString(8)
 
-	for i := 0; i < feedbackUploadMaxRetries; i++ {
+	for i := 0; i < feedbackUploadMaxAttempts; i++ {
 
 		uploadURL := transferURLs.Select(i)
 		if uploadURL == nil {
@@ -197,19 +197,20 @@ func SendFeedback(ctx context.Context, configJson, diagnosticsJson, uploadPath s
 			if ctx.Err() != nil {
 				// Input context has completed
 				return errors.TraceMsg(err,
-					fmt.Sprintf("feedback upload attempt %d cancelled", i+1))
+					fmt.Sprintf("feedback upload attempt %d/%d cancelled", i+1, feedbackUploadMaxAttempts))
 			}
 			// Do not sleep after the last attempt
-			if i+1 < feedbackUploadMaxRetries {
+			if i+1 < feedbackUploadMaxAttempts {
 				// Log error, sleep and then retry
 				timeUntilRetry := prng.Period(feedbackUploadMinRetryDelay, feedbackUploadMaxRetryDelay)
 				NoticeWarning(
-					"feedback upload attempt %d failed (retry in %.0fs): %s",
-					i+1, timeUntilRetry.Seconds(), errors.Trace(err))
+					"feedback upload attempt %d/%d failed (retry in %.0fs): %s",
+					i+1, feedbackUploadMaxAttempts, timeUntilRetry.Seconds(), errors.Trace(err))
 				select {
 				case <-ctx.Done():
 					return errors.TraceNew(
-						fmt.Sprintf("feedback upload attempt %d cancelled before attempt", i+2))
+						fmt.Sprintf("feedback upload attempt %d/%d cancelled before attempt",
+							i+2, feedbackUploadMaxAttempts))
 				case <-time.After(timeUntilRetry):
 				}
 				continue