v3: context value added in Before missing in After when subcommands are involved

[bystones]

When adding values to a context in a Before function that value is not present in the After function when a subcommand was run. Without the subcommand the value is present as expected.

The following test demonstrates that behavior and should pass:

func TestCommand_Run_BeforeReturnNewContextSubcommand(t *testing.T) {
        var receivedValFromAction, receivedValFromAfter string
        type key string

        bkey := key("bkey")

        cmd := &Command{
                Name: "bar",
                Before: func(ctx context.Context, cmd *Command) (context.Context, error) {
                        return context.WithValue(ctx, bkey, "bval"), nil
                },
                After: func(ctx context.Context, cmd *Command) error {
                        if val := ctx.Value(bkey); val == nil {
                                return errors.New("bkey value not found")
                        } else {
                                receivedValFromAfter = val.(string)
                        }
                        return nil
                },
                Commands: []*Command{
                        {
                                Name: "baz",
                                Action: func(ctx context.Context, cmd *Command) error {
                                        if val := ctx.Value(bkey); val == nil {
                                                return errors.New("bkey value not found")
                                        } else {
                                                receivedValFromAction = val.(string)
                                        }
                                        return nil
                                },
                        },
                },
        }

        require.NoError(t, cmd.Run(buildTestContext(t), []string{"bar", "baz"}))
        require.Equal(t, "bval", receivedValFromAfter)
        require.Equal(t, "bval", receivedValFromAction)
}

This test is a slight variation of an already existing test:

cli/command_test.go

Lines 320 to 352 in efab65a

	func TestCommand_Run_BeforeReturnNewContext(t *testing.T) {
	var receivedValFromAction, receivedValFromAfter string
	type key string
	bkey := key("bkey")
	cmd := &Command{
	Name: "bar",
	Before: func(ctx context.Context, cmd *Command) (context.Context, error) {
	return context.WithValue(ctx, bkey, "bval"), nil
	},
	Action: func(ctx context.Context, cmd *Command) error {
	if val := ctx.Value(bkey); val == nil {
	return errors.New("bkey value not found")
	} else {
	receivedValFromAction = val.(string)
	}
	return nil
	},
	After: func(ctx context.Context, cmd *Command) error {
	if val := ctx.Value(bkey); val == nil {
	return errors.New("bkey value not found")
	} else {
	receivedValFromAfter = val.(string)
	}
	return nil
	},
	}
	require.NoError(t, cmd.Run(buildTestContext(t), []string{"foo", "bar"}))
	require.Equal(t, "bval", receivedValFromAfter)
	require.Equal(t, "bval", receivedValFromAction)
	}

[dearchap]

@bystones Thanks for the test case. Yes its correct. It appears that the defer statement utilizes the context that is passed to subcommand rather than the one that is created by Before. Tricky to solve. Any ideas ?

[bystones]

My first attempt was this: bystones@acb130d
Moving the defer of the After function down and doing the same "chain" thing as the Before function fixes the test and keeps "make test" green as well.

The problem with that fix attempt is that it changes behavior around other error handling which seems to be currently untested. For example the following test passes before that change and breaks after: bystones@e308fae

I'm not familiar with the internals of the library and seeing the above I'm not very confident that I can come up with a fix that doesn't silently introduce a bunch of changes around the interaction with panics/errors and the After function.

[bystones]

@dearchap I might have found a solution, see PR #2108