Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Command context is not properly passed on to sub-commands #1109

Open
lewisparma opened this issue Apr 27, 2020 · 5 comments
Open

Command context is not properly passed on to sub-commands #1109

lewisparma opened this issue Apr 27, 2020 · 5 comments
Labels
triage/needs-info Needs more investigation from maintainers or more info from the issue provider

Comments

@lewisparma
Copy link

lewisparma commented Apr 27, 2020

When using 'ExecuteContext' on the parent command, the context is not properly passed on to sub commands. It's passed on only the first time, then on the following invocations the first passed value is used. The culprit seems to be this section from commands.go:

// We have to pass global context to children command
// if context is present on the parent command.
if cmd.ctx == nil {
cmd.ctx = c.ctx
}

the parent context should always be passed on to the child command, not just when the child context is null.

@github-actions
Copy link

This issue is being marked as stale due to a long period of inactivity

@github-actions
Copy link

This issue is being marked as stale due to a long period of inactivity

@utku-caglayan
Copy link

Here is a workaround for anyone struggling with this:

type CobraMutableCtx struct {
	Internal context.Context
}
func (c *CobraMutableCtx) Deadline() (deadline time.Time, ok bool) {
	return c.Internal.Deadline()
}
func (c *CobraMutableCtx) Done() <-chan struct{} {
	return c.Internal.Done()
}
func (c *CobraMutableCtx) Err() error {
	return c.Internal.Err()
}
func (c *CobraMutableCtx) Value(key interface{}) interface{} {
	return c.Internal.Value(key)
}
...
var cobraCtx CobraMutableCtx
cobraCtx.Internal = context.TODO() // your context
cobraCmd.ExecuteContext(&cobraCtx) // this will be passed to subcommands
cobraCtx.Internal, cmdCancel = context.WithCancel(ctx) // your context
cmdCancel()
cobraCmd.ExecuteContext(&cobraCtx) // this time subcommands will use cancelled context

@johnSchnake
Copy link
Collaborator

Is there any counterargument for the context not being passed to the child when non-nil?

Unfortunately it seems like it would be a breaking change for anyone who already worked around it or began to depend on this code flow.

Out of curiosity, whats the situation with the multiple invocations? Is it tests that are causing the problem? I wonder if that was part of the reason why the context wasn't to be overwritten: so that you could specify it manually to avoid the inheritance.

@jpmcb jpmcb added triage/needs-info Needs more investigation from maintainers or more info from the issue provider and removed needs investigation labels Feb 23, 2022
@marckhouzam
Copy link
Collaborator

Very similar to #1469

radhus added a commit to einride/aip-cli-go that referenced this issue Oct 11, 2022
Sometimes cmd.Context() returns nil, specifically in this project when
called with `help completion` as arguments.

This seems to be a known issue, see e.g. [1] and [2].

Use workaround from [3] try fetch context from parents, otherwise
default to empty Config, to avoid crashing.

[1]: spf13/cobra#1469
[2]: spf13/cobra#1109
[3]: spf13/cobra#1469 (comment)
radhus added a commit to einride/aip-cli-go that referenced this issue Oct 11, 2022
Sometimes cmd.Context() returns nil, specifically in this project when
called with `help completion` as arguments.

This seems to be a known issue, see e.g. [1] and [2].

Use workaround from [3] try fetch context from parents, otherwise
default to empty Config, to avoid crashing.

[1]: spf13/cobra#1469
[2]: spf13/cobra#1109
[3]: spf13/cobra#1469 (comment)
radhus added a commit to einride/aip-cli-go that referenced this issue Oct 11, 2022
Sometimes cmd.Context() returns nil, specifically in this project when
called with `help completion` as arguments.

This seems to be a known issue, see e.g. [1] and [2].

Use workaround from [3] try fetch context from parents, otherwise
default to empty Config, to avoid crashing.

[1]: spf13/cobra#1469
[2]: spf13/cobra#1109
[3]: spf13/cobra#1469 (comment)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
triage/needs-info Needs more investigation from maintainers or more info from the issue provider
Projects
None yet
Development

No branches or pull requests

6 participants