CODEWITHSUNDEEP
go
anlogg
anlogg

Reputation: 1090

Too many arguments to return error

I've been pulling my hair out as to why this code throws the error:

package util

import (
   "path/filepath"
   "sync"

   "github.com/go-ini/ini"
)

// ConfigMap is map for config values
type ConfigMap struct {
    LogPath         string
    PublicDir       string
    SessionName     string
    Debug           bool
    DBUsersHost     string
    DBUsersName     string
    DBUsersUsername string
    DBUsersPassword string
}

var once sync.Once

// Config loads and return config object
func Config() (*ConfigMap, error) {
    once.Do(func() {
        // Find the location of the app.conf file
        configFilePath, err := filepath.Abs("../build/app.conf")
        if err != nil {
            return nil, err
        }

        // Load app.conf
        cfg, err := ini.Load(configFilePath)
        if err != nil {
            return nil, err
        }

        // Get app mode
        mode, err := AppMode()
        if err != nil {
            return nil, err
        }

        c := &ConfigMap{}
        err = cfg.Section(mode).MapTo(c)
        if err != nil {
            return nil, err
        }

        return c, err
    })
}

As you can see, the pairing is exactly correct. Each return code returns &ConfigMap and err and the function signature matches it. What am I missing?

Upvotes: 1

Views: 6630

Answers (2)

icza
icza

Reputation: 417612

You pass an anonymous function value to once.Do() (which is Once.Do()), and the return statements are inside that. Which means those return statements want to return from the anonymous function, but it doesn't have any return values:

func Config() (*ConfigMap, error) {
    once.Do(func() {
        // You can't return any values here, only this works:
        return
    })

    // And you do need to return something here:
    return &ConfigMap{}, nil
}

What you may do is create global variables matching the return values of Config(), and the anonymous function should store the values in them. And in Config() you may return the values of these global variables.

var cm *ConfigMap
var cmErr error

func Config() (*ConfigMap, error) {
    once.Do(func() {
        // load config, and store, e.g.
        cm, cmErr = &ConfigMap{}, nil
    })

    return cm, cmErr
}

Do we really need global variables? Since the values returned by Config() are produced by the anonymous function passed to once.Do() which is guaranteed to run only once, yes, you need to store them somewhere to be able to return them multiple times, even when the anonymous function is not called / run anymore (on subsequent calls to Config()).

Question: May there be a data race here?

If Config() is called from multiple goroutines, at least one will write the global variables cm and cmErr, and all will read them. So it's rightful to ask this question.

But the answer is no, the above code is safe. The global variables cm and cmErr are only written once, and this happens before they could be read. Because once.Do() blocks until the anonymous function returns. If Config() (and thus once.Do()) is called simultaneously from multiple goroutines, all will block until the anonymous function completes (once only), and reading the variables can happen only after the first write. And since the anonymous function will not run anymore, no more writes will happen.

Upvotes: 9

Paul Hankin
Paul Hankin

Reputation: 58241

You're calling return nil, err and similar from the nested func() inside your once.Do. Conversely, you're not returning from the actual function.

Instead, you can structure your code like this:

func newConfig() (*Config, error) {
    configFilePath, err := filepath.Abs("../build/app.conf")
    if err != nil {
        return nil, err
    }

    // Load app.conf
    cfg, err := ini.Load(configFilePath)
    if err != nil {
        return nil, err
    }

    // Get app mode
    mode, err := AppMode()
    if err != nil {
        return nil, err
    }

    c := &ConfigMap{}
    err = cfg.Section(mode).MapTo(c)
    if err != nil {
        return nil, err
    }

    return c, err
}

// Cached config and any error.
var (
   cachedConfig *Config
   cachedConfigErr error
)

func Config() (*Config, error) {
  once.Do(func() {
    cachedConfig, cachedConfigErr = newConfig()
  })
  return cachedConfig, cachedConfigErr
}

Upvotes: 1

Related Questions