diff --git a/internal/dispatcher/dispatcher.go b/internal/dispatcher/dispatcher.go index ad6a9ea..f156364 100644 --- a/internal/dispatcher/dispatcher.go +++ b/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 } } diff --git a/internal/dispatcher/dispatcher_test.go b/internal/dispatcher/dispatcher_test.go index 2069994..2e0d781 100644 --- a/internal/dispatcher/dispatcher_test.go +++ b/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() diff --git a/pkg/connector/inet/inet.go b/pkg/connector/inet/inet.go index 7fc6e6b..b340b51 100644 --- a/pkg/connector/inet/inet.go +++ b/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) {