chore: make AutoTokenRefresh a boolean pointer#74
Conversation
|
💬 Discussion in Slack: #pr-review-go-sdk-74-chore-make-autotokenrefresh-a-boolean-pointer Posted by Review Police — reviews, comments, new commits, and CI failures will stream into this channel. |
|
| Filename | Overview |
|---|---|
| client.go | Converts AutoTokenRefresh from bool to *bool, removes the reflect.Bool default-override path, and adds autoTokenRefreshEnabled helper; fixes the stated bug cleanly with two minor usability follow-ups. |
Comments Outside Diff (1)
-
client.go, line 504-507 (link)Goroutine keeps idling after
AutoTokenRefreshis disabled at runtimeWhen a client is created with
AutoTokenRefreshenabled (nil or&true), the goroutine is started unconditionally. IfUpdateConfigurationis later called withAutoTokenRefresh: Bool(false), the goroutine falls into theelsebranch and loops indefinitely on a 1-second sleep — it can only be terminated by cancelling the original context. This is now a reachable path (the old default-override bug prevented it), so callers who callUpdateConfigurationto turn off auto-refresh expecting the background worker to stop will be surprised. At minimum the comment on this branch should note that the goroutine stays alive until context cancellation.
Reviews (1): Last reviewed commit: "Update client.go" | Re-trigger Greptile
The AutoTokenRefresh=false option does nothing currently because the default value is being overwritten to true when its false. I've converted AutoTokenRefresh to a boolean pointer to fix this at the cost of changing the struct signature for folks already using the
AutoTokenRefreshparameter