Skip to content

Commit 857dbd3

Browse files
committed
fix(analytics): reconcile every auth login error path to a terminal event
runAPIKeyLogin and runOAuthLogin had several early-return error branches (malformed credentials file, save/clear failures, PKCE/state generation, loopback startup, authorize-URL build, response encoding) that returned without emitting AuthLoginCompleted or AuthLoginFailed, leaving their AuthLoginStarted event dangling with no outcome. Rather than instrumenting each scattered return site, wrap both runners with a deferred catch-all: a per-call `reported` flag is set at each existing AuthLoginCompleted/AuthLoginFailed call, and a defer fires AuthLoginFailed(method, "internal_error") only when the function returns an error and nothing else already reported an outcome. New error paths added later are covered automatically instead of silently reintroducing the gap.
1 parent 5bff15c commit 857dbd3

1 file changed

Lines changed: 39 additions & 2 deletions

File tree

cmd/heygen/auth_login.go

Lines changed: 39 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -309,13 +309,29 @@ func errorsAsPickerCanceled(err error) bool {
309309
// cleared so the file holds at most one of api_key / oauth (the
310310
// single-credential normalization invariant). Pre-this-change users
311311
// with both blocks self-heal on their next login.
312-
func runAPIKeyLogin(cmd *cobra.Command, ctx *cmdContext) error {
312+
func runAPIKeyLogin(cmd *cobra.Command, ctx *cmdContext) (err error) {
313+
// reported tracks whether this attempt already emitted a terminal
314+
// analytics event (AuthLoginCompleted/AuthLoginFailed) on one of the
315+
// explicitly-instrumented paths below. The deferred check is a
316+
// safety net for every OTHER error return in this function (e.g. a
317+
// future early return nobody remembers to instrument): without it,
318+
// the AuthLoginStarted event the caller already fired would never
319+
// reconcile.
320+
reported := false
321+
defer func() {
322+
if err != nil && !reported {
323+
loginAnalytics.AuthLoginFailed("api_key", "internal_error")
324+
}
325+
}()
326+
313327
key, err := readAPIKey(cmd.InOrStdin(), cmd.ErrOrStderr())
314328
if err != nil {
329+
reported = true
315330
loginAnalytics.AuthLoginFailed("api_key", "api_key_aborted")
316331
return err
317332
}
318333
if key == "" {
334+
reported = true
319335
loginAnalytics.AuthLoginFailed("api_key", "api_key_invalid_input")
320336
if stdinIsTerminalFunc() {
321337
return clierrors.NewUsage(
@@ -416,6 +432,7 @@ func runAPIKeyLogin(cmd *cobra.Command, ctx *cmdContext) error {
416432
if err != nil {
417433
return clierrors.New(fmt.Sprintf("failed to encode response: %v", err))
418434
}
435+
reported = true
419436
loginAnalytics.AuthLoginCompleted("api_key")
420437
if display := userInfo.DisplayName(); display != "" {
421438
fmt.Fprintf(cmd.ErrOrStderr(), "Logged in as %s\n", display)
@@ -425,7 +442,21 @@ func runAPIKeyLogin(cmd *cobra.Command, ctx *cmdContext) error {
425442

426443
// runOAuthLogin drives the browser + PKCE + loopback dance and persists
427444
// the resulting tokens.
428-
func runOAuthLogin(cmd *cobra.Command, ctx *cmdContext, cfg oauthLoginConfig) error {
445+
func runOAuthLogin(cmd *cobra.Command, ctx *cmdContext, cfg oauthLoginConfig) (err error) {
446+
// reported tracks whether this attempt already emitted a terminal
447+
// analytics event (AuthLoginCompleted/AuthLoginFailed) on one of the
448+
// explicitly-instrumented paths below. The deferred check is a
449+
// safety net for every OTHER error return in this function (e.g. a
450+
// future early return nobody remembers to instrument): without it,
451+
// the AuthLoginStarted event the caller already fired would never
452+
// reconcile.
453+
reported := false
454+
defer func() {
455+
if err != nil && !reported {
456+
loginAnalytics.AuthLoginFailed("oauth", "internal_error")
457+
}
458+
}()
459+
429460
// Headless-shell fast-fail: when explicit --oauth lands in a shell
430461
// with no TTY on stdin AND the environment opts out of opening a
431462
// browser (BROWSER=none or HEYGEN_NO_BROWSER=1), the callback can
@@ -434,6 +465,7 @@ func runOAuthLogin(cmd *cobra.Command, ctx *cmdContext, cfg oauthLoginConfig) er
434465
// guard when cfg.OpenBrowser is injected (test path drives the
435466
// callback synthetically). (N1)
436467
if cfg.OpenBrowser == nil && isHeadlessOAuthShell() {
468+
reported = true
437469
loginAnalytics.AuthLoginFailed("oauth", "headless_shell")
438470
return clierrors.NewUsage(
439471
"cannot complete browser OAuth flow in a headless shell " +
@@ -492,21 +524,25 @@ func runOAuthLogin(cmd *cobra.Command, ctx *cmdContext, cfg oauthLoginConfig) er
492524
var loopbackResult oauth.LoopbackResult
493525
select {
494526
case <-cmdCtx.Done():
527+
reported = true
495528
loginAnalytics.AuthLoginFailed("oauth", "oauth_timeout")
496529
return clierrors.New(fmt.Sprintf("oauth: timed out waiting for browser callback: %v", cmdCtx.Err()))
497530
case loopbackResult = <-results:
498531
}
499532
if loopbackResult.Err != nil {
533+
reported = true
500534
loginAnalytics.AuthLoginFailed("oauth", "oauth_flow_error")
501535
return clierrors.New(loopbackResult.Err.Error())
502536
}
503537

504538
tok, err := oc.ExchangeAuthorizationCode(cmdCtx, loopbackResult.Code, verifier, loopbackResult.RedirectURI)
505539
if err != nil {
540+
reported = true
506541
loginAnalytics.AuthLoginFailed("oauth", "token_exchange_failed")
507542
return clierrors.New(fmt.Sprintf("oauth: token exchange failed: %v", err))
508543
}
509544
if tok.AccessToken == "" {
545+
reported = true
510546
loginAnalytics.AuthLoginFailed("oauth", "token_exchange_failed")
511547
return clierrors.New("oauth: token endpoint returned no access_token")
512548
}
@@ -599,6 +635,7 @@ func runOAuthLogin(cmd *cobra.Command, ctx *cmdContext, cfg oauthLoginConfig) er
599635
if err != nil {
600636
return clierrors.New(fmt.Sprintf("failed to encode response: %v", err))
601637
}
638+
reported = true
602639
loginAnalytics.AuthLoginCompleted("oauth")
603640
if display := userInfo.DisplayName(); display != "" {
604641
fmt.Fprintf(cmd.ErrOrStderr(), "Logged in as %s\n", display)

0 commit comments

Comments
 (0)