panic on self built gitea version v1.0.2

[ston1th]

• Gitea version (or commit ref): v1.0.2
• Operating system: linux amd64

Description

Hello Giteas,

I built the latest gitea version (v1.0.2) and got a panic from the locale library:

panic: fail to set message file(en-US): open conf/locale/locale_en-US.ini: no such file or directory

goroutine 1 [running]:
code.gitea.io/gitea/vendor/github.com/go-macaron/i18n.initLocales(0xc4202a8278, 0x0, 0xdd2713, 0xb, 0xc420453f20, 0xddc70b, 0x12, 0xc420126160, 0x15, 0x15, ...)
	/home/marius/go/src/code.gitea.io/gitea/vendor/github.com/go-macaron/i18n/i18n.go:57 +0x5ff
code.gitea.io/gitea/vendor/github.com/go-macaron/i18n.I18n(0xc420084b40, 0x1, 0x1, 0x0, 0xc420453f20)
	/home/marius/go/src/code.gitea.io/gitea/vendor/github.com/go-macaron/i18n/i18n.go:158 +0xee
code.gitea.io/gitea/cmd.newMacaron(0xc42000f040)
	/home/marius/go/src/code.gitea.io/gitea/cmd/web.go:125 +0x715
code.gitea.io/gitea/cmd.runWeb(0xc42000f040, 0x0, 0xc42000f000)
	/home/marius/go/src/code.gitea.io/gitea/cmd/web.go:160 +0x6f
code.gitea.io/gitea/vendor/github.com/urfave/cli.HandleAction(0xcbdae0, 0xe07d78, 0xc42000f040, 0xc420016900, 0x0)
	/home/marius/go/src/code.gitea.io/gitea/vendor/github.com/urfave/cli/app.go:471 +0xb9
code.gitea.io/gitea/vendor/github.com/urfave/cli.Command.Run(0xdc69b6, 0x3, 0x0, 0x0, 0x0, 0x0, 0x0, 0xde1ae2, 0x16, 0x0, ...)
	/home/marius/go/src/code.gitea.io/gitea/vendor/github.com/urfave/cli/command.go:191 +0xb4b
code.gitea.io/gitea/vendor/github.com/urfave/cli.(*App).Run(0xc42010a9c0, 0xc42000a160, 0x2, 0x2, 0x0, 0x0)
	/home/marius/go/src/code.gitea.io/gitea/vendor/github.com/urfave/cli/app.go:241 +0x65f
main.main()
	/home/marius/go/src/code.gitea.io/gitea/main.go:42 +0x377

I think the library (go-macaron/i18n) should handle or return such an error instead of panicking.

Anyway, if I'm using the prebuilt binary (github release page) everything works fine, though.

Thanks,
ston1th

[morrildl]

Building with TAGS="bindata" (to embed assets) might get you past this.

[ston1th]

Thanks, that worked.

Sadly, the panic call in the i18n library, seems unfixable without an API change.

[strk]

Are you saying 1.0.2 needs to be built with TAGS="bindata" or it would panic ? If that's the case this ticket should be reopened and considered high priority

[ston1th]

I don't have a conf/* directory on my server.
The i18n library seems to expect this file (and maybe more files) in there: conf/locale/locale_en-US.ini

So basically you are right, in a setup where this file is missing and v1.0.2 is not built with TAGS="bindata" it will panic.

This line is the problem's source:
https://github.com/go-gitea/gitea/blob/master/vendor/github.com/go-macaron/i18n/i18n.go#L57

In my opinion, a library never should call panic, but return an error.
The problem is, that it seems not fixable without breaking the API, but maybe im wrong there.

[ston1th]

I opened an issue to address this: go-macaron/i18n#5

I didn't test if there are more components affected, when building without bindata.

A quick fix (HEAD at v1.0.2):

diff --git a/cmd/web.go b/cmd/web.go
index 9776fdc1..596f28f4 100644
--- a/cmd/web.go
+++ b/cmd/web.go
@@ -115,14 +115,18 @@ func newMacaron() *macaron.Macaron {
                }
        }
 
-       m.Use(i18n.I18n(i18n.Options{
+       i18nHandler, err := i18n.I18n(i18n.Options{
                SubURL:      setting.AppSubURL,
                Files:       localFiles,
                Langs:       setting.Langs,
                Names:       setting.Names,
                DefaultLang: "en-US",
                Redirect:    true,
-       }))
+       })
+       if err != nil {
+               log.Fatal(4, "Failed to initialize locale middleware: %v", err)
+       }
+       m.Use(i18nHandler)
        m.Use(cache.Cacher(cache.Options{
                Adapter:       setting.CacheAdapter,
                AdapterConfig: setting.CacheConn,
diff --git a/vendor/github.com/go-macaron/i18n/i18n.go b/vendor/github.com/go-macaron/i18n/i18n.go
index 3b5b1b83..cf795d57 100644
--- a/vendor/github.com/go-macaron/i18n/i18n.go
+++ b/vendor/github.com/go-macaron/i18n/i18n.go
@@ -33,7 +33,7 @@ func Version() string {
 }
 
 // initLocales initializes language type list and Accept-Language header matcher.
-func initLocales(opt Options) language.Matcher {
+func initLocales(opt Options) (language.Matcher, error) {
        tags := make([]language.Tag, len(opt.Langs))
        for i, lang := range opt.Langs {
                tags[i] = language.Raw.Make(lang)
@@ -54,10 +54,10 @@ func initLocales(opt Options) language.Matcher {
 
                err := i18n.SetMessageWithDesc(lang, opt.Names[i], locale, custom...)
                if err != nil && err != i18n.ErrLangAlreadyExist {
-                       panic(fmt.Errorf("fail to set message file(%s): %v", lang, err))
+                       return nil, err
                }
        }
-       return language.NewMatcher(tags)
+       return language.NewMatcher(tags), nil
 }
 
 // A Locale describles the information of localization.
@@ -153,9 +153,12 @@ type LangType struct {
 // I18n is a middleware provides localization layer for your application.
 // Paramenter langs must be in the form of "en-US", "zh-CN", etc.
 // Otherwise it may not recognize browser input.
-func I18n(options ...Options) macaron.Handler {
+func I18n(options ...Options) (macaron.Handler, error) {
        opt := prepareOptions(options)
-       m := initLocales(opt)
+       m, err := initLocales(opt)
+       if err != nil {
+               return nil, err
+       }
        return func(ctx *macaron.Context) {
                isNeedRedir := false
                hasCookie := false
@@ -221,5 +224,5 @@ func I18n(options ...Options) macaron.Handler {
                if opt.Redirect && isNeedRedir {
                        ctx.Redirect(opt.SubURL + ctx.Req.RequestURI[:strings.Index(ctx.Req.RequestURI, "?")])
                }
-       }
+       }, nil
 }

Another (ugly) "fix" would be to use recover:

package main

import (
        "fmt"
)

func willPanic() int {
        panic("panic")
        return 1
}

func panicWrapper(err *error) int {
        defer func() {
                *err = fmt.Errorf("%v", recover())
        }()
        return willPanic()
}

func main() {
        var err error
        i := panicWrapper(&err)
        fmt.Println(i, err)
}

[ston1th]

Following Unknown's response (go-macaron/i18n#5) this will not be fixed in the main repository.

To avoid the ugly workarounds obove, I would suggest to create a fork of https://github.com/go-macaron/i18n, break the API and fix the problem in the library itself.

What do you guys think?

[lunny]

@tboerger any idea?

[tboerger]

Just set https://github.com/go-macaron/i18n/blob/master/i18n.go#L78 properly on our side.

[ston1th]

But, this would not prevent the application to panic on file system errors per se?

For example:
If the directory/file has wrong access priviledges or they are missing, than gitea would panic (and same as in my case no error would be written to the logs).

[tboerger]

But, this would not prevent the application to panic on file system errors per se?

For example:
If the directory/file has wrong access priviledges or they are missing, than gitea would panic (and same as in my case no error would be written to the logs).

Of course not, but we can not swap everything directly.

[klingtnet]

I get the same error for gitea build from the latest master with the bindata tag enabled.

$ ./gitea web
2017/04/17 16:19:47 [W] Custom config '/tmp/custom/conf/app.ini' not found, ignore this if you're running first time
2017/04/17 16:19:47 [T] Custom path: /tmp/custom
2017/04/17 16:19:47 [T] Log path: /tmp/log
2017/04/17 16:19:47 [I] Gitea v1.1.0+dev
2017/04/17 16:19:47 [I] Log Mode: Console(Trace)
2017/04/17 16:19:47 [I] XORM Log Mode: Console(Trace)
2017/04/17 16:19:47 [I] Cache Service Enabled
2017/04/17 16:19:47 [I] Session Service Enabled
2017/04/17 16:19:47 [I] Run Mode: Development
panic: fail to set message file(en-US): open conf/locale/locale_en-US.ini: no such file or directory