Repository: grpc/grpc-go
File: internal/xds/xdsclient/pool.go (Go)

Code before:
	p.mu.Lock()
	defer p.mu.Unlock()

	if c := p.clients[name]; c != nil {
		c.incrRef()
		return c, sync.OnceFunc(func() { p.clientRefCountedClose(name) }), nil
	}

	var (
		config *bootstrap.Config
		err    error
	)

	if bConfig != nil {
		config = bConfig
	} else {
		config, err = p.getConfiguration()
		if err != nil {
			return nil, nil, fmt.Errorf("xds: failed to read xDS bootstrap config from env vars:  %v", err)
		}
		if config == nil {
			// If the environment variables are not set, then fallback bootstrap
			// configuration should be set before attempting to create an xDS client,
			// else xDS client creation will fail.
			config = p.fallbackConfig
		}
	}

	if config == nil {
		return nil, nil, fmt.Errorf("failed to read xDS bootstrap config from env vars: bootstrap environment variables (%q or %q) not defined and fallback config not set", envconfig.XDSBootstrapFileNameEnv, envconfig.XDSBootstrapFileContentEnv)
	}

	c, err := newClientImpl(config, metricsRecorder, name, watchExpiryTimeout)
	if err != nil {
		return nil, nil, err
	}
	if logger.V(2) {
		c.logger.Infof("Created client with name %q and bootstrap configuration:\n %s", name, config)
	}
	p.clients[name] = c
	xdsClientImplCreateHook(name)

	logger.Infof("xDS node ID: %s", config.Node().GetId())
	return c, sync.OnceFunc(func() { p.clientRefCountedClose(name) }), nil
}


Review comment (bug):
The idiomatic way of doing this in Go (which shortens things up a bit) is as follows:

```suggestion
	config := bConfig
	if config == nil {
		var err error
		config, err = p.getConfiguration()
		if err != nil {
			return nil, nil, fmt.Errorf("xds: failed to read xDS bootstrap config from env vars:  %v", err)
		}
		if config == nil {
			// If the environment variables are not set, then fallback bootstrap
			// configuration should be set before attempting to create an xDS client,
			// else xDS client creation will fail.
			config = p.fallbackConfig
		}
```

Code after:
func (p *Pool) newRefCounted(name string, metricsRecorder estats.MetricsRecorder, watchExpiryTimeout time.Duration, bConfig *bootstrap.Config) (*clientImpl, func(), error) {
	p.mu.Lock()
	defer p.mu.Unlock()

	if c := p.clients[name]; c != nil {
		c.incrRef()
		return c, sync.OnceFunc(func() { p.clientRefCountedClose(name) }), nil
	}

	config := bConfig
	if config == nil {
		var err error
		config, err = p.getConfiguration()
		if err != nil {
			return nil, nil, fmt.Errorf("xds: failed to read xDS bootstrap config from env vars:  %v", err)
		}
		if config == nil {
			// If the environment variables are not set, then fallback bootstrap
			// configuration should be set before attempting to create an xDS client,
			// else xDS client creation will fail.
			config = p.fallbackConfig
		}
	}

	if config == nil {
		return nil, nil, fmt.Errorf("failed to read xDS bootstrap config from env vars: bootstrap environment variables (%q or %q) not defined and fallback config not set", envconfig.XDSBootstrapFileNameEnv, envconfig.XDSBootstrapFileContentEnv)
	}

	c, err := newClientImpl(config, metricsRecorder, name, watchExpiryTimeout)
	if err != nil {
		return nil, nil, err
	}
	if logger.V(2) {
		c.logger.Infof("Created client with name %q and bootstrap configuration:\n %s", name, config)
	}
	p.clients[name] = c
	xdsClientImplCreateHook(name)

	logger.Infof("xDS node ID: %s", config.Node().GetId())
	return c, sync.OnceFunc(func() { p.clientRefCountedClose(name) }), nil
}