Merge pull request #1154 from wakatime/bugfix/unhandled-default-params

Fix unhandled default params
wakatime · Jan 6, 2025 · 02eefc9 · 02eefc9
2 parents 92addb5 + 15f70f2
commit 02eefc9
Show file tree

Hide file tree

Showing 12 changed files with 279 additions and 32 deletions.
diff --git a/USAGE.md b/USAGE.md
@@ -111,7 +111,7 @@ some/submodule/name = new project name
 | hostname                       | Optional name of local machine. By default, auto-detects the local machine’s hostname. | _string_ | |
 | log_file                       | Optional log file path. | _filepath_ | `~/.wakatime/wakatime.log` |
 | import_cfg                     | Optional path to another wakatime.cfg file to import. If set it will overwrite values loaded from $WAKATIME_HOME/.wakatime.cfg file. | _filepath_ | |
-| metrics                        | When set, collects metrics usage in '~/.wakatime/metrics' folder. For further reference visit <https://go.dev/blog/pprof>. | _bool_ | `false` |
+| metrics                        | When set, collects metrics usage in `~/.wakatime/metrics` folder. For further reference visit <https://go.dev/blog/pprof>. | _bool_ | `false` |
 | guess_language                 | When `true`, enables detecting programming language from file contents. | _bool_ | `false` |
 
 ### Project Map Section

diff --git a/cmd/heartbeat/heartbeat.go b/cmd/heartbeat/heartbeat.go
@@ -43,9 +43,8 @@ func Run(ctx context.Context, v *viper.Viper) (int, error) {
 	if err != nil {
 		var errauth api.ErrAuth
 
-		// api.ErrAuth represents an error when parsing api key.
-		// Save heartbeats to offline db even when api key invalid.
-		// It avoids losing heartbeats when api key is invalid.
+		// api.ErrAuth represents an error when parsing api key or timeout.
+		// Save heartbeats to offline db when api.ErrAuth as it avoids losing heartbeats.
 		if errors.As(err, &errauth) {
 			if err := offlinecmd.SaveHeartbeats(ctx, v, nil, queueFilepath); err != nil {
 				logger.Errorf("failed to save heartbeats to offline queue: %s", err)

diff --git a/cmd/params/params.go b/cmd/params/params.go
@@ -22,6 +22,7 @@ import (
 	"github.com/wakatime/wakatime-cli/pkg/heartbeat"
 	"github.com/wakatime/wakatime-cli/pkg/ini"
 	"github.com/wakatime/wakatime-cli/pkg/log"
+	"github.com/wakatime/wakatime-cli/pkg/offline"
 	"github.com/wakatime/wakatime-cli/pkg/output"
 	"github.com/wakatime/wakatime-cli/pkg/project"
 	"github.com/wakatime/wakatime-cli/pkg/regex"
@@ -296,10 +297,10 @@ func LoadAPIParams(ctx context.Context, v *viper.Viper) (API, error) {
 		}
 	}
 
-	var timeout time.Duration
+	timeout := api.DefaultTimeoutSecs
 
 	if timeoutSecs, ok := vipertools.FirstNonEmptyInt(v, "timeout", "settings.timeout"); ok {
-		timeout = time.Duration(timeoutSecs) * time.Second
+		timeout = timeoutSecs
 	}
 
 	return API{
@@ -312,7 +313,7 @@ func LoadAPIParams(ctx context.Context, v *viper.Viper) (API, error) {
 		Plugin:           vipertools.GetString(v, "plugin"),
 		ProxyURL:         proxyURL,
 		SSLCertFilepath:  sslCertFilepath,
-		Timeout:          timeout,
+		Timeout:          time.Duration(timeout) * time.Second,
 		URL:              apiURL.String(),
 	}, nil
 }
@@ -658,17 +659,26 @@ func LoadOfflineParams(ctx context.Context, v *viper.Viper) Offline {
 
 	logger := log.Extract(ctx)
 
-	rateLimit, _ := vipertools.FirstNonEmptyInt(v, "heartbeat-rate-limit-seconds", "settings.heartbeat_rate_limit_seconds")
-	if rateLimit < 0 {
-		logger.Warnf("argument --heartbeat-rate-limit-seconds must be zero or a positive integer number, got %d", rateLimit)
+	rateLimit := offline.RateLimitDefaultSeconds
 
-		rateLimit = 0
+	if rateLimitSecs, ok := vipertools.FirstNonEmptyInt(v,
+		"heartbeat-rate-limit-seconds",
+		"settings.heartbeat_rate_limit_seconds"); ok {
+		rateLimit = rateLimitSecs
+
+		if rateLimit < 0 {
+			logger.Warnf(
+				"argument --heartbeat-rate-limit-seconds must be zero or a positive integer number, got %d",
+				rateLimit,
+			)
+
+			rateLimit = 0
+		}
 	}
 
 	syncMax := v.GetInt("sync-offline-activity")
 	if syncMax < 0 {
 		logger.Warnf("argument --sync-offline-activity must be zero or a positive integer number, got %d", syncMax)
-
 		syncMax = 0
 	}
 
@@ -1100,7 +1110,7 @@ func (p Offline) String() string {
 	}
 
 	return fmt.Sprintf(
-		"disabled: %t, last sent at: '%s', print max: %d, num rate limit: %d, num sync max: %d",
+		"disabled: %t, last sent at: '%s', print max: %d, rate limit: %s, num sync max: %d",
 		p.Disabled,
 		lastSentAt,
 		p.PrintMax,

diff --git a/cmd/params/params_test.go b/cmd/params/params_test.go
@@ -20,14 +20,15 @@ import (
 	"github.com/wakatime/wakatime-cli/pkg/heartbeat"
 	inipkg "github.com/wakatime/wakatime-cli/pkg/ini"
 	"github.com/wakatime/wakatime-cli/pkg/log"
+	"github.com/wakatime/wakatime-cli/pkg/offline"
 	"github.com/wakatime/wakatime-cli/pkg/output"
 	"github.com/wakatime/wakatime-cli/pkg/project"
 	"github.com/wakatime/wakatime-cli/pkg/regex"
-	"gopkg.in/ini.v1"
 
 	"github.com/spf13/viper"
 	"github.com/stretchr/testify/assert"
 	"github.com/stretchr/testify/require"
+	"gopkg.in/ini.v1"
 )
 
 func TestLoadHeartbeatParams_AlternateProject(t *testing.T) {
@@ -1781,6 +1782,50 @@ func TestLoadAPIParams_Timeout_FromConfig(t *testing.T) {
 	assert.Equal(t, 10*time.Second, params.Timeout)
 }
 
+func TestLoadAPIParams_Timeout_Zero(t *testing.T) {
+	v := viper.New()
+	v.Set("key", "00000000-0000-4000-8000-000000000000")
+	v.Set("timeout", 0)
+
+	params, err := cmdparams.LoadAPIParams(context.Background(), v)
+	require.NoError(t, err)
+
+	assert.Zero(t, params.Timeout)
+}
+
+func TestLoadAPIParams_Timeout_Default(t *testing.T) {
+	v := viper.New()
+	v.Set("key", "00000000-0000-4000-8000-000000000000")
+	v.SetDefault("timeout", api.DefaultTimeoutSecs)
+
+	params, err := cmdparams.LoadAPIParams(context.Background(), v)
+	require.NoError(t, err)
+
+	assert.Equal(t, time.Duration(api.DefaultTimeoutSecs)*time.Second, params.Timeout)
+}
+
+func TestLoadAPIParams_Timeout_NegativeNumber(t *testing.T) {
+	v := viper.New()
+	v.Set("key", "00000000-0000-4000-8000-000000000000")
+	v.Set("timeout", 0)
+
+	params, err := cmdparams.LoadAPIParams(context.Background(), v)
+	require.NoError(t, err)
+
+	assert.Zero(t, params.Timeout)
+}
+
+func TestLoadAPIParams_Timeout_NonIntegerValue(t *testing.T) {
+	v := viper.New()
+	v.Set("key", "00000000-0000-4000-8000-000000000000")
+	v.Set("timeout", "invalid")
+
+	params, err := cmdparams.LoadAPIParams(context.Background(), v)
+	require.NoError(t, err)
+
+	assert.Equal(t, time.Duration(api.DefaultTimeoutSecs)*time.Second, params.Timeout)
+}
+
 func TestLoadOfflineParams_Disabled_ConfigTakesPrecedence(t *testing.T) {
 	v := viper.New()
 	v.Set("disable-offline", false)
@@ -1832,7 +1877,7 @@ func TestLoadOfflineParams_RateLimit_FromConfig(t *testing.T) {
 
 func TestLoadOfflineParams_RateLimit_Zero(t *testing.T) {
 	v := viper.New()
-	v.Set("heartbeat-rate-limit-seconds", "0")
+	v.Set("heartbeat-rate-limit-seconds", 0)
 
 	params := cmdparams.LoadOfflineParams(context.Background(), v)
 
@@ -1841,11 +1886,11 @@ func TestLoadOfflineParams_RateLimit_Zero(t *testing.T) {
 
 func TestLoadOfflineParams_RateLimit_Default(t *testing.T) {
 	v := viper.New()
-	v.SetDefault("heartbeat-rate-limit-seconds", 20)
+	v.SetDefault("heartbeat-rate-limit-seconds", offline.RateLimitDefaultSeconds)
 
 	params := cmdparams.LoadOfflineParams(context.Background(), v)
 
-	assert.Equal(t, time.Duration(20)*time.Second, params.RateLimit)
+	assert.Equal(t, time.Duration(offline.RateLimitDefaultSeconds)*time.Second, params.RateLimit)
 }
 
 func TestLoadOfflineParams_RateLimit_NegativeNumber(t *testing.T) {
@@ -1863,7 +1908,7 @@ func TestLoadOfflineParams_RateLimit_NonIntegerValue(t *testing.T) {
 
 	params := cmdparams.LoadOfflineParams(context.Background(), v)
 
-	assert.Zero(t, params.RateLimit)
+	assert.Equal(t, time.Duration(offline.RateLimitDefaultSeconds)*time.Second, params.RateLimit)
 }
 
 func TestLoadOfflineParams_LastSentAt(t *testing.T) {
@@ -1889,7 +1934,7 @@ func TestLoadOfflineParams_LastSentAt_Err(t *testing.T) {
 
 func TestLoadOfflineParams_LastSentAtFuture(t *testing.T) {
 	v := viper.New()
-	lastSentAt := time.Now().Add(time.Duration(2) * time.Hour)
+	lastSentAt := time.Now().Add(2 * time.Hour)
 	v.Set("internal.heartbeats_last_sent_at", lastSentAt.Format(inipkg.DateFormat))
 
 	params := cmdparams.LoadOfflineParams(context.Background(), v)
@@ -1984,6 +2029,7 @@ func TestLoadAPIParams_APIKey(t *testing.T) {
 		t.Run(name, func(t *testing.T) {
 			v := viper.New()
 			v.Set("hostname", "my-computer")
+			v.Set("timeout", 0)
 			v.Set("key", test.ViperAPIKey)
 			v.Set("settings.api_key", test.ViperAPIKeyConfig)
 			v.Set("settings.apikey", test.ViperAPIKeyConfigOld)
@@ -2193,6 +2239,7 @@ func TestLoadAPIParams_APIUrl(t *testing.T) {
 		t.Run(name, func(t *testing.T) {
 			v := viper.New()
 			v.Set("hostname", "my-computer")
+			v.Set("timeout", 0)
 			v.Set("key", "00000000-0000-4000-8000-000000000000")
 			v.Set("api-url", test.ViperAPIUrl)
 			v.Set("apiurl", test.ViperAPIUrlOld)
@@ -2232,6 +2279,7 @@ func TestLoadAPIParams_Url_InvalidFormat(t *testing.T) {
 func TestLoadAPIParams_BackoffAt(t *testing.T) {
 	v := viper.New()
 	v.Set("hostname", "my-computer")
+	v.Set("timeout", 0)
 	v.Set("key", "00000000-0000-4000-8000-000000000000")
 	v.Set("internal.backoff_at", "2021-08-30T18:50:42-03:00")
 	v.Set("internal.backoff_retries", "3")
@@ -2255,19 +2303,15 @@ func TestLoadAPIParams_BackoffAtErr(t *testing.T) {
 	v := viper.New()
 	v.Set("hostname", "my-computer")
 	v.Set("key", "00000000-0000-4000-8000-000000000000")
+	v.Set("timeout", 0)
 	v.Set("internal.backoff_at", "2021-08-30")
 	v.Set("internal.backoff_retries", "2")
 
 	params, err := cmdparams.LoadAPIParams(context.Background(), v)
 	require.NoError(t, err)
 
-	assert.Equal(t, cmdparams.API{
-		BackoffAt:      time.Time{},
-		BackoffRetries: 2,
-		Key:            "00000000-0000-4000-8000-000000000000",
-		URL:            "https://api.wakatime.com/api/v1",
-		Hostname:       "my-computer",
-	}, params)
+	assert.Equal(t, 2, params.BackoffRetries)
+	assert.Empty(t, params.BackoffAt)
 }
 
 func TestLoadAPIParams_BackoffAtFuture(t *testing.T) {
@@ -2658,14 +2702,14 @@ func TestOffline_String(t *testing.T) {
 		Disabled:   true,
 		LastSentAt: lastSentAt,
 		PrintMax:   6,
-		RateLimit:  15,
+		RateLimit:  time.Duration(15) * time.Second,
 		SyncMax:    12,
 	}
 
 	assert.Equal(
 		t,
 		"disabled: true, last sent at: '2021-08-30T18:50:42-03:00', print max: 6,"+
-			" num rate limit: 15, num sync max: 12",
+			" rate limit: 15s, num sync max: 12",
 		offline.String(),
 	)
 }

diff --git a/cmd/run_internal_test.go b/cmd/run_internal_test.go
@@ -415,6 +415,10 @@ func TestParseConfigFiles(t *testing.T) {
 	assert.Equal(t,
 		"2006-01-02T15:04:05Z07:00",
 		v.GetString("internal.backoff_at"))
+	assert.Equal(t,
+		"2025-01-05T22:21:51Z03:00",
+		v.GetString("internal.heartbeats_last_sent_at"),
+	)
 }
 
 func TestParseConfigFiles_MissingAPIKey(t *testing.T) {

diff --git a/cmd/testdata/.wakatime-internal.cfg b/cmd/testdata/.wakatime-internal.cfg
@@ -1,3 +1,4 @@
 [internal]
 backoff_retries  = 1
 backoff_at       = 2006-01-02T15:04:05Z07:00
+heartbeats_last_sent_at = 2025-01-05T22:21:51Z03:00
diff --git a/main_test.go b/main_test.go
@@ -249,6 +249,98 @@ func TestSendHeartbeats_SecondaryApiKey(t *testing.T) {
 	assert.Eventually(t, func() bool { return numCalls == 1 }, time.Second, 50*time.Millisecond)
 }
 
+func TestSendHeartbeats_Timeout(t *testing.T) {
+	apiURL, router, close := setupTestServer()
+	defer close()
+
+	ctx := context.Background()
+
+	// to avoid race condition
+	wg := sync.WaitGroup{}
+	wg.Add(1)
+
+	var numCalls int
+
+	go func() {
+		router.HandleFunc("/users/current/heartbeats.bulk", func(w http.ResponseWriter, _ *http.Request) {
+			defer wg.Done()
+
+			numCalls++
+
+			time.Sleep(1010 * time.Millisecond) // simulate a slow server to force a timeout
+
+			// write response
+			f, err := os.Open("testdata/api_heartbeats_response.json")
+			require.NoError(t, err)
+
+			w.WriteHeader(http.StatusCreated)
+			_, err = io.Copy(w, f)
+			require.NoError(t, err)
+		})
+	}()
+
+	tmpDir := t.TempDir()
+
+	offlineQueueFile, err := os.CreateTemp(tmpDir, "")
+	require.NoError(t, err)
+
+	defer offlineQueueFile.Close()
+
+	offlineQueueFileLegacy, err := os.CreateTemp(tmpDir, "")
+	require.NoError(t, err)
+
+	// close the file to avoid "The process cannot access the file because it is being used by another process" error
+	offlineQueueFileLegacy.Close()
+
+	tmpConfigFile, err := os.CreateTemp(tmpDir, "wakatime.cfg")
+	require.NoError(t, err)
+
+	defer tmpConfigFile.Close()
+
+	tmpInternalConfigFile, err := os.CreateTemp(tmpDir, "wakatime-internal.cfg")
+	require.NoError(t, err)
+
+	defer tmpInternalConfigFile.Close()
+
+	projectFolder, err := filepath.Abs(".")
+	require.NoError(t, err)
+
+	out := runWakatimeCliExpectErr(
+		t,
+		exitcode.ErrGeneric,
+		"--api-url", apiURL,
+		"--key", "00000000-0000-4000-8000-000000000000",
+		"--config", tmpConfigFile.Name(),
+		"--internal-config", tmpInternalConfigFile.Name(),
+		"--entity", "testdata/main.go",
+		"--cursorpos", "12",
+		"--offline-queue-file", offlineQueueFile.Name(),
+		"--offline-queue-file-legacy", offlineQueueFileLegacy.Name(),
+		"--line-additions", "123",
+		"--line-deletions", "456",
+		"--lineno", "42",
+		"--lines-in-file", "100",
+		"--time", "1585598059",
+		"--hide-branch-names", ".*",
+		"--project", "wakatime-cli",
+		"--project-folder", projectFolder,
+		"--timeout", "1", // very short timeout to force a timeout error
+		"--write",
+		"--verbose",
+	)
+
+	assert.Empty(t, out)
+
+	offlineCount, err := offline.CountHeartbeats(ctx, offlineQueueFile.Name())
+	require.NoError(t, err)
+
+	assert.Equal(t, 1, offlineCount)
+
+	wg.Wait()
+
+	assert.Eventually(t, func() bool { return numCalls == 1 }, time.Second, 50*time.Millisecond)
+}
+
 func TestSendHeartbeats_ExtraHeartbeats(t *testing.T) {
 	apiURL, router, close := setupTestServer()
 	defer close()
@@ -455,7 +547,6 @@ func TestSendHeartbeats_ExtraHeartbeats_SyncLegacyOfflineActivity(t *testing.T)
 		"--offline-queue-file-legacy", offlineQueueFileLegacy.Name(),
 		"--lineno", "42",
 		"--lines-in-file", "100",
-		"--heartbeat-rate-limit-seconds", "0",
 		"--time", "1585598059",
 		"--hide-branch-names", ".*",
 		"--write",