Skip to content

Commit 3d50ed5

Browse files
committed
Keep the auth remedy for the credential sent when several filings fail
A 401 while filing several to-dos ended in a report that kept only each failure's text, so App.Err no longer saw the SDK error and left the bare "Run: basecamp auth login" in the hint. Under --profile work that logs into the default profile; under BASECAMP_TOKEN no login helps. runFilings now settles the remedy on each failure before the report drops the cause, through App.WithAuthRemedy, which App.Err already used. The report stays non-retryable. A 401 does not stop the remaining to-dos. Every to-do is read before any is filed, so a rejected credential fails there, before the loop; one rejected mid-run is refused before anything is filed for each later to-do too, and the report already asks to run those again.
1 parent 68f198e commit 3d50ed5

3 files changed

Lines changed: 43 additions & 4 deletions

File tree

‎internal/appctx/context.go‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -283,7 +283,7 @@ func (a *App) Err(err error) error {
283283
}
284284

285285
// Print the error response
286-
if outputErr := a.Output.Err(a.withAuthRemedy(err), opts...); outputErr != nil {
286+
if outputErr := a.Output.Err(a.WithAuthRemedy(err), opts...); outputErr != nil {
287287
return outputErr
288288
}
289289

@@ -295,7 +295,7 @@ func (a *App) Err(err error) error {
295295
return nil
296296
}
297297

298-
// withAuthRemedy replaces the generic login hint on an API 401 with one that
298+
// WithAuthRemedy replaces the generic login hint on an API 401 with one that
299299
// fits the credential the request actually sent. The SDK classifies a 401
300300
// far from the profile and the environment, so its conversion can only say
301301
// "basecamp auth login": under an active profile that command would store
@@ -305,7 +305,7 @@ func (a *App) Err(err error) error {
305305
// the credential manager's own failures name the profile themselves, and
306306
// come from stored-credential operations (auth refresh, auth token
307307
// --stored) that ignore the environment token by design.
308-
func (a *App) withAuthRemedy(err error) error {
308+
func (a *App) WithAuthRemedy(err error) error {
309309
var sdkErr *basecamp.Error
310310
e := output.AsError(err)
311311
if e.Code != output.CodeAuth || !errors.As(err, &sdkErr) || (e.Hint != "" && !strings.HasPrefix(e.Hint, output.DefaultAuthHint)) {

‎internal/commands/filing.go‎

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -280,7 +280,10 @@ func runFilings(cmd *cobra.Command, app *appctx.App, scope followUpScope, filing
280280
filed, outcome, err := fileRecording(cmd.Context(), app, f)
281281
var e *output.Error
282282
if err != nil {
283-
e = output.AsError(err)
283+
// The report keeps only each failure's text, so a 401's remedy
284+
// for the credential sent is settled here, while the SDK error
285+
// that App.Err would read is still attached.
286+
e = output.AsError(app.WithAuthRemedy(err))
284287
if worst == nil || outcome > worstOf {
285288
worst, worstOf = e, outcome
286289
}

‎internal/commands/todos_filing_test.go‎

Lines changed: 36 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -640,3 +640,39 @@ func TestFilingFollowUpsNameTheProfileAndAnAccountGivenThisRun(t *testing.T) {
640640
})
641641
}
642642
}
643+
644+
// A 401 partway through several filings must keep the remedy that fits the
645+
// credential sent, as a single filing's does: under a profile, a login to the
646+
// default one fixes nothing, and under BASECAMP_TOKEN no login does.
647+
func TestTodosFilingSeveralKeepsTheAuthRemedyForTheCredentialSent(t *testing.T) {
648+
for name, tc := range map[string]struct {
649+
envToken string
650+
want string
651+
}{
652+
"a profile": {"", "Run: basecamp auth login -P work"},
653+
"an env token": {"bc_at_env", "BASECAMP_TOKEN is set"},
654+
} {
655+
t.Run(name, func(t *testing.T) {
656+
t.Setenv("BASECAMP_TOKEN", tc.envToken)
657+
shortFilingWait(t)
658+
transport := &mockTodoFilingTransport{postStatus: http.StatusUnauthorized}
659+
app, buf := newTestAppWithTransport(t, transport)
660+
app.Config.ActiveProfile = "work"
661+
662+
err := executeCommand(NewTodosCmd(), app, "copy", "456", "457", "--to-list", "901")
663+
require.Error(t, err)
664+
assert.Len(t, transport.posts, 2)
665+
require.NoError(t, app.Err(err))
666+
667+
var envelope struct {
668+
Code string `json:"code"`
669+
Hint string `json:"hint"`
670+
}
671+
require.NoError(t, json.Unmarshal(buf.Bytes(), &envelope), buf.String())
672+
assert.Equal(t, output.CodeAuth, envelope.Code)
673+
assert.Contains(t, envelope.Hint, tc.want)
674+
assert.NotRegexp(t, `basecamp auth login(\n|$)`, envelope.Hint, "the bare login hint is replaced")
675+
assert.Contains(t, envelope.Hint, "run the command again for #456, #457 alone")
676+
})
677+
}
678+
}

0 commit comments

Comments
 (0)