Skip to content
Merged
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
46 changes: 46 additions & 0 deletions daemon/cc_info_timer.go
Original file line number Diff line number Diff line change
Expand Up @@ -432,6 +432,9 @@ func (s *CCInfoTimerService) fetchRateLimit(ctx context.Context) {
s.rateLimitCache.fetchedAt = time.Now()
s.rateLimitCache.mu.Unlock()

// Send usage data to server for push notification scheduling (fire-and-forget)
go s.sendAnthropicUsageToServer(ctx, usage)
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🔴 Fire-and-forget goroutine uses a context that is cancelled immediately after spawning

The sendAnthropicUsageToServer goroutine is spawned at line 436 with the same ctx that was passed to fetchRateLimit. However, the callers of fetchRateLimit (at lines 171-173 and 194-196) create a timeout context with defer cancel() in the enclosing anonymous function. When fetchRateLimit returns synchronously, cancel() fires, cancelling the context. The newly spawned goroutine then tries to make an HTTP request with this already-cancelled context, which will fail immediately.

Root Cause and Impact

The call chain is:

  1. timerLoop spawns an anonymous goroutine (line 166-174)
  2. Inside it: ctx, cancel := context.WithTimeout(context.Background(), 10*time.Second) then defer cancel()
  3. s.fetchRateLimit(ctx) is called synchronously
  4. Inside fetchRateLimit, at line 436: go s.sendAnthropicUsageToServer(ctx, usage) — this spawns a goroutine with the same ctx
  5. fetchRateLimit returns → defer cancel() fires → ctx is cancelled
  6. sendAnthropicUsageToServer calls model.SendHTTPRequestJSON which uses http.NewRequestWithContext(ctx, ...) at model/api.base.go:50 — this request will fail immediately with context canceled

Impact: The usage data will never be successfully sent to the server. The fire-and-forget feature is completely non-functional. Every attempt will log "Failed to send anthropic usage to server" with a context cancellation error.

Suggested change
go s.sendAnthropicUsageToServer(ctx, usage)
go s.sendAnthropicUsageToServer(context.Background(), usage)
Open in Devin Review

Was this helpful? React with 👍 or 👎 to provide feedback.


slog.Debug("Anthropic rate limit updated",
slog.Float64("5h", usage.FiveHourUtilization),
slog.Float64("7d", usage.SevenDayUtilization))
Expand Down Expand Up @@ -472,6 +475,49 @@ func (s *CCInfoTimerService) fetchUserProfile(ctx context.Context) {
slog.Debug("User profile fetched", slog.String("login", profile.FetchUser.Login))
}

// sendAnthropicUsageToServer sends the Anthropic usage data to the ShellTime server
// for scheduling push notifications when rate limits reset.
func (s *CCInfoTimerService) sendAnthropicUsageToServer(ctx context.Context, usage *AnthropicRateLimitData) {
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

The function sendAnthropicUsageToServer could benefit from input validation. Before sending the data, validate the usage parameter to ensure that the FiveHourUtilization, SevenDayUtilization, FiveHourResetsAt, and SevenDayResetsAt fields contain valid data. This can prevent unexpected errors or incorrect data being sent to the server.

if s.config.Token == "" {
return
}

type usageBucket struct {
Utilization float64 `json:"utilization"`
ResetsAt string `json:"resets_at"`
}
type usagePayload struct {
FiveHour usageBucket `json:"five_hour"`
SevenDay usageBucket `json:"seven_day"`
}

payload := usagePayload{
FiveHour: usageBucket{
Utilization: usage.FiveHourUtilization,
ResetsAt: usage.FiveHourResetsAt,
},
SevenDay: usageBucket{
Utilization: usage.SevenDayUtilization,
ResetsAt: usage.SevenDayResetsAt,
},
}

err := model.SendHTTPRequestJSON(model.HTTPRequestOptions[usagePayload, any]{
Context: ctx,
Endpoint: model.Endpoint{
Token: s.config.Token,
APIEndpoint: s.config.APIEndpoint,
},
Method: "POST",
Path: "/api/v1/anthropic-usage",
Payload: payload,
Timeout: 5 * time.Second,
})
if err != nil {
slog.Warn("Failed to send anthropic usage to server", slog.Any("err", err))
}
}
Comment on lines +478 to +519
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

Consider adding more robust error handling. While the current implementation logs a warning if sending usage data fails, it might be beneficial to implement a retry mechanism with exponential backoff to handle transient network issues. This would improve the reliability of sending usage data to the server.


// GetCachedRateLimit returns a copy of the cached rate limit data, or nil if not available.
func (s *CCInfoTimerService) GetCachedRateLimit() *AnthropicRateLimitData {
s.rateLimitCache.mu.RLock()
Expand Down