Fix retransmission logic, 503 error

Fixes the dispatcher retransmission logic so that the dispatcher
remembers if one of its attempts might have succeeded (even if it never
received a response from the vehicle).

Categorizes an HTTP 503 code as a possible success, since the server may
return this status code if it relays a command to a vehicle but does not
hear back.

internal/dispatcher/dispatcher.go

@@ -397,20 +397,39 @@ func (d *Dispatcher) Send(ctx context.Context, message *universal.RoutableMessag
 		}
 	}()
 
+	possibleSuccess := false
 	for {
 		err = d.conn.Send(ctx, encodedMessage)
 		if err == nil {
 			return resp, nil
 		}
-		if !protocol.ShouldRetry(err) {
-			log.Warning("[%02x] Terminal transmission error: %s", message.GetUuid(), err)
-			return nil, err
+		log.Debug("[%02x] Received error %s", message.GetUuid(), err)
+
+		// The vehicle will de-duplicate exact copies of encodedMessage and re-send the cached
+		// response, so we're free to resend without risk of executing the action multiple times.
+		// However, we need to notify callers higher up on the stack that a command may have
+		// succeeded so that they do not retry with, e.g., a higher counter value.
+		if protocol.MayHaveSucceeded(err) {
+			log.Debug("[%02x] Command may have succeed (vehicle discards duplicates)", message.GetUuid())
+			possibleSuccess = true
+		}
+		if !protocol.Temporary(err) {
+			log.Warning("[%02x] Error is terminal", message.GetUuid())
+			return nil, &protocol.CommandError{
+				Err:               err,
+				PossibleSuccess:   possibleSuccess,
+				PossibleTemporary: false,
+			}
 		}
-		log.Debug("[%02x] Retrying transmission after error: %s", message.GetUuid(), err)
 		select {
 		case <-ctx.Done():
-			return nil, &protocol.CommandError{Err: ctx.Err(), PossibleSuccess: false, PossibleTemporary: true}
+			return nil, &protocol.CommandError{
+				Err:               ctx.Err(),
+				PossibleSuccess:   possibleSuccess,
+				PossibleTemporary: true,
+			}
 		case <-time.After(d.conn.RetryInterval()):
+			log.Debug("[%02x] Retrying transmission", message.GetUuid())
 			continue
 		}
 	}

internal/dispatcher/dispatcher_test.go

@@ -612,7 +612,7 @@ func TestVehicleUnreachable(t *testing.T) {
 	ctx, cancel := context.WithTimeout(context.Background(), quiescentDelay)
 	defer cancel()
 
-	if _, err := dispatcher.Send(ctx, testCommand(), connector.AuthMethodNone); err != errTimeout {
+	if _, err := dispatcher.Send(ctx, testCommand(), connector.AuthMethodNone); !errors.Is(err, errTimeout) {
 		t.Errorf("Unexpected error: %s", err)
 	}
 }
@@ -706,7 +706,7 @@ func TestRetrySend(t *testing.T) {
 		conn.EnqueueSendError(&protocol.CommandError{Err: errTimeout, PossibleSuccess: false, PossibleTemporary: true})
 	}
 	errFoo := errors.New("not enough pylons")
-	conn.EnqueueSendError(&protocol.CommandError{Err: errFoo, PossibleSuccess: true, PossibleTemporary: true})
+	conn.EnqueueSendError(&protocol.CommandError{Err: errFoo, PossibleSuccess: true, PossibleTemporary: false})
 
 	req := testCommand()
 	rsp, err := dispatcher.Send(ctx, req, connector.AuthMethodNone)
@@ -718,6 +718,63 @@ func TestRetrySend(t *testing.T) {
 	}
 }
 
+func TestReportPossibleSuccess(t *testing.T) {
+	dispatcher, conn := getTestSetup(t)
+	defer conn.Close()
+
+	const errCount = 3
+	ctx, cancel := context.WithTimeout(context.Background(), time.Second)
+	defer cancel()
+
+	errFoo := errors.New("not enough pylons")
+	conn.EnqueueSendError(&protocol.CommandError{Err: errFoo, PossibleSuccess: true, PossibleTemporary: true})
+
+	for i := 0; i < errCount; i++ {
+		conn.EnqueueSendError(&protocol.CommandError{Err: errTimeout, PossibleSuccess: false, PossibleTemporary: true})
+	}
+
+	conn.EnqueueSendError(&protocol.CommandError{Err: errTimeout, PossibleSuccess: false, PossibleTemporary: false})
+
+	req := testCommand()
+	rsp, err := dispatcher.Send(ctx, req, connector.AuthMethodNone)
+	if err == nil {
+		rsp.Close()
+	}
+	if !errors.Is(err, errTimeout) {
+		t.Errorf("Unexpected error: %s", err)
+	}
+	if !protocol.MayHaveSucceeded(err) {
+		t.Errorf("Dispatcher did not flag possible success: %+v", err)
+	}
+}
+
+func TestReportCertainFailure(t *testing.T) {
+	dispatcher, conn := getTestSetup(t)
+	defer conn.Close()
+
+	const errCount = 3
+	ctx, cancel := context.WithTimeout(context.Background(), time.Second)
+	defer cancel()
+
+	for i := 0; i < errCount; i++ {
+		conn.EnqueueSendError(&protocol.CommandError{Err: errTimeout, PossibleSuccess: false, PossibleTemporary: true})
+	}
+
+	conn.EnqueueSendError(&protocol.CommandError{Err: errTimeout, PossibleSuccess: false, PossibleTemporary: false})
+
+	req := testCommand()
+	rsp, err := dispatcher.Send(ctx, req, connector.AuthMethodNone)
+	if err == nil {
+		rsp.Close()
+	}
+	if !errors.Is(err, errTimeout) {
+		t.Errorf("Unexpected error: %s", err)
+	}
+	if protocol.MayHaveSucceeded(err) {
+		t.Errorf("Dispatcher flagged as possible success: %+v", err)
+	}
+}
+
 func TestSendTimeout(t *testing.T) {
 	dispatcher, conn := getTestSetup(t)
 	defer conn.Close()

pkg/connector/inet/inet.go

@@ -70,15 +70,13 @@ func (e *HttpError) MayHaveSucceeded() bool {
 	if e.Code >= 400 && e.Code < 500 {
 		return false
 	}
-	return e.Code != http.StatusServiceUnavailable
+	return true
 }
 
 func (e *HttpError) Temporary() bool {
-	return e.Code == http.StatusServiceUnavailable ||
-		e.Code == http.StatusGatewayTimeout ||
-		e.Code == http.StatusRequestTimeout ||
-		e.Code == http.StatusMisdirectedRequest ||
-		e.Code == http.StatusTooManyRequests
+	// See https://developer.tesla.com/docs/tesla-fleet-api#response-codes
+	return e.Code == http.StatusServiceUnavailable || // 503 - Did not receive vehicle response
+		e.Code == http.StatusMisdirectedRequest // 421 - Wrong domain (client corrects domain based on server response)
 }
 
 func SendFleetAPICommand(ctx context.Context, client *http.Client, userAgent, authHeader string, url string, command interface{}) ([]byte, error) {