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

fx.Options and fx.Module behave differently #918

Closed
djmitche opened this issue Aug 9, 2022 · 4 comments
Closed

fx.Options and fx.Module behave differently #918

djmitche opened this issue Aug 9, 2022 · 4 comments
Labels

Comments

@djmitche
Copy link

djmitche commented Aug 9, 2022

Describe the bug

It appears that fx.Invoke's that are in an fx.Module do not run until after all fx.Invoke's outside the module.

This is not documented in fx.Module, and fx.Invoke's docs say

Invoke registers functions that are executed eagerly on application start. Arguments for these invocations are built using the constructors registered by Provide. Passing multiple Invoke options appends the new invocations to the application's existing list.

Unlike constructors, invocations are always executed, and they're always run in order. Invocations may have any number of returned values. If the final returned object is an error, it indicates whether the operation was successful. All other returned values are discarded.

emphasis on in order :)

To Reproduce
https://go.dev/play/p/SP93TiMiji9

package main

import (
	"context"
	"fmt"

	"go.uber.org/fx"
)

func main() {
	app := fx.New(
		fx.Invoke(func() { fmt.Printf("Outer invoke 1\n") }),
		fx.Module(
			"mod",
			fx.Invoke(func() { fmt.Printf("Module invoke\n") }),
		),
		fx.Invoke(func() { fmt.Printf("Outer invoke 2\n") }),
	)
	app.Start(context.Background())
	app.Stop(context.Background())
}

)

Actual behavior

Outer invoke 1
Outer invoke 2
Module invoke

Expected behavior

Outer invoke 1
Module invoke
Outer invoke 2

Additional context
Changing fx.Module to fx.Options (and removing the "mod") makes this behave as expected.

@djmitche
Copy link
Author

djmitche commented Aug 9, 2022

In the use-case I'm working on, I want to write

app := fx.New(
  someModule,
  anotherModule,
  fx.Invoke(func(lc fx.Lifecycle) {
    lc.Append(fx.Hook{OnStart: ...})
  }),
)

and know that, because it's the last invocation, that fx.Hook is the last thing started in the lifecycle. As it is, everything in the modules is starting after my top-level hook.

djmitche added a commit to DataDog/dd-agent-comp-experiments that referenced this issue Aug 12, 2022
This refactors the command-running utilities to take the fx.Options as
variadic arguments, so that fxapps.OneShot can add its own fx.Option.

It uses a bit of reflection to convince Fx to gather the arguments to a
function without actually calling that function immediately, allowing
the oneShot function to run after `app.Start` has finished, when we're
sure that all components have started.  This works around the startup
ordering bug in uber-go/fx#918.
@sywhang
Copy link
Contributor

sywhang commented Aug 19, 2022

This is a valid issue, thanks for reporting this.

Tracking internal issue: GO-1591.

@sywhang sywhang added the bug label Aug 19, 2022
@sywhang
Copy link
Contributor

sywhang commented Aug 19, 2022

BTW, just to clarify what the issue is - the issue isn't that fx.Options and fx.Module behaves differently. They do behave differently because they're meant for doing different things :)

There are two issues that this issue is tracking:

  1. Lack of documentation on the invoke order when fx.Module is involved.
  2. fx.Module invokes are run after the parent-level invokes.

sywhang added a commit to sywhang/fx that referenced this issue Aug 19, 2022
Currently, fx.Module's invokes are run after all the invokes provided
at the parent level are invoked.

This makes it difficult for module consumers to control the precise
invoke orders.

This changes the invoke order to run all the invokes in the child
module before running any of the invokes in the parent module, and
clarify that order in the documentation for Invoke.

Solves uber-go#918.
Refs GO-1591.
sywhang added a commit to sywhang/fx that referenced this issue Aug 19, 2022
Currently, fx.Module's invokes are run after all the invokes provided
at the parent level are invoked.

This makes it difficult for module consumers to control the precise
invoke orders.

This changes the invoke order to run all the invokes in the child
module before running any of the invokes in the parent module, and
clarify that order in the documentation for Invoke.

Solves uber-go#918.
Refs GO-1591.
sywhang added a commit that referenced this issue Aug 19, 2022
* Fix Invoke order when fx.Module is used

Currently, fx.Module's invokes are run after all the invokes provided
at the parent level are invoked.

This makes it difficult for module consumers to control the precise
invoke orders.

This changes the invoke order to run all the invokes in the child
module before running any of the invokes in the parent module, and
clarify that order in the documentation for Invoke.

Solves #918.
Refs GO-1591.

* fix indentation in code sample
@sywhang
Copy link
Contributor

sywhang commented Aug 19, 2022

Fixed with #925.

@sywhang sywhang closed this as completed Aug 19, 2022
sywhang added a commit to sywhang/fx that referenced this issue Oct 11, 2022
* Fix Invoke order when fx.Module is used

Currently, fx.Module's invokes are run after all the invokes provided
at the parent level are invoked.

This makes it difficult for module consumers to control the precise
invoke orders.

This changes the invoke order to run all the invokes in the child
module before running any of the invokes in the parent module, and
clarify that order in the documentation for Invoke.

Solves uber-go#918.
Refs GO-1591.

* fix indentation in code sample
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Development

No branches or pull requests

2 participants