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

refac: DRY up New, reuse *All methods, delete modules slice #1258

Merged
merged 2 commits into from
Jan 9, 2025
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
22 changes: 6 additions & 16 deletions app.go
Original file line number Diff line number Diff line change
Expand Up @@ -304,7 +304,6 @@ type App struct {

container *dig.Container
root *module
modules []*module

// Timeouts used
startTimeout time.Duration
Expand Down Expand Up @@ -446,7 +445,6 @@ func New(opts ...Option) *App {
log: logger,
trace: []string{fxreflect.CallerStack(1, 2)[0].String()},
}
app.modules = append(app.modules, app.root)

for _, opt := range opts {
opt.apply(app.root)
Expand Down Expand Up @@ -475,10 +473,7 @@ func New(opts ...Option) *App {
}

app.container = dig.New(containerOptions...)

for _, m := range app.modules {
m.build(app, app.container)
}
app.root.build(app, app.container)

// Provide Fx types first to increase the chance a custom logger
// can be successfully built in the face of unrelated DI failure.
Expand All @@ -490,31 +485,26 @@ func New(opts ...Option) *App {
})
app.root.provide(provide{Target: app.shutdowner, Stack: frames})
app.root.provide(provide{Target: app.dotGraph, Stack: frames})
app.root.provideAll()

for _, m := range app.modules {
m.provideAll()
}

// Run decorators before executing any Invokes -- including the one
// inside constructCustomLogger.
// Run decorators before executing any Invokes
// (including the ones inside installAllEventLoggers).
app.err = multierr.Append(app.err, app.root.decorateAll())

// If you are thinking about returning here after provides: do not (just yet)!
// If a custom logger was being used, we're still buffering messages.
// We'll want to flush them to the logger.

// custom app logger will be initialized by the root module.
for _, m := range app.modules {
m.constructAllCustomLoggers()
}
app.root.installAllEventLoggers()

// This error might have come from the provide loop above. We've
// already flushed to the custom logger, so we can return.
if app.err != nil {
return app
}

if err := app.root.executeInvokes(); err != nil {
if err := app.root.invokeAll(); err != nil {
app.err = err

if dig.CanVisualizeError(err) {
Expand Down
17 changes: 8 additions & 9 deletions module.go
Original file line number Diff line number Diff line change
Expand Up @@ -253,11 +253,11 @@ func (m *module) supply(p provide) {
}

// Constructs custom loggers for all modules in the tree
func (m *module) constructAllCustomLoggers() {
func (m *module) installAllEventLoggers() {
if m.logConstructor != nil {
if buffer, ok := m.log.(*logBuffer); ok {
// default to parent's logger if custom logger constructor fails
if err := m.constructCustomLogger(buffer); err != nil {
if err := m.installEventLogger(buffer); err != nil {
m.app.err = multierr.Append(m.app.err, err)
m.log = m.fallbackLogger
buffer.Connect(m.log)
Expand All @@ -269,12 +269,11 @@ func (m *module) constructAllCustomLoggers() {
}

for _, mod := range m.modules {
mod.constructAllCustomLoggers()
mod.installAllEventLoggers()
}
}

// Mirroring the behavior of app.constructCustomLogger
func (m *module) constructCustomLogger(buffer *logBuffer) (err error) {
func (m *module) installEventLogger(buffer *logBuffer) (err error) {
p := m.logConstructor
fname := fxreflect.FuncName(p.Target)
defer func() {
Expand All @@ -297,23 +296,23 @@ func (m *module) constructCustomLogger(buffer *logBuffer) (err error) {
})
}

func (m *module) executeInvokes() error {
func (m *module) invokeAll() error {
for _, m := range m.modules {
if err := m.executeInvokes(); err != nil {
if err := m.invokeAll(); err != nil {
return err
}
}

for _, invoke := range m.invokes {
if err := m.executeInvoke(invoke); err != nil {
if err := m.invoke(invoke); err != nil {
return err
}
}

return nil
}

func (m *module) executeInvoke(i invoke) (err error) {
func (m *module) invoke(i invoke) (err error) {
fnName := fxreflect.FuncName(i.Target)
m.log.LogEvent(&fxevent.Invoking{
FunctionName: fnName,
Expand Down
Loading