Fiber: 🐛 Fiber's flags are part of the printed usage

[doingodswork]

Fiber version

v0.12.1

Issue description

I use flag.Parse in my app to parse things like bind address, port and more app-specific settings. The app is offered for self-hosting to others, so it's not an app I just run myself on a server. With the switch to Fiber two flags show up when doing app -h that I never defined myself : -prefork and -child. _This would confuse the users of my app_.

I found a workaround for this, but it's kind of ugly and I was wondering if there's an "official" recommended approach to deal with this.

The code that probably causes this is here: https://github.com/gofiber/fiber/blob/ec514589405a7ab637f97e8ce410a76a9433f5f4/prefork.go#L28-L29

There was a GitHub issue where flag.Parse vs. scanning os.Args was discussed, which concluded with keeping to use os.Args scanning, but somehow that got changed later anyway: https://github.com/gofiber/fiber/issues/301

Code snippet

Reproducible with this:

package main

import (
    "flag"

    "github.com/gofiber/fiber"
)

var (
    port = flag.Int("port", 8080, "Port")
)

func main() {
    flag.Parse()

    app := fiber.New()
    app.Get("/", func(c *fiber.Ctx) {
        c.Send("Hello, World!")
    })
    app.Listen(*port)
}

Then do go run . -h or go build -o fiber-flags && ./fiber-flags -h.

This is printed:

Usage of /path/to/fiber-flags:
-port int
Port (default 8080)
-prefork
is child proc
-prefork-child
use prefork

But of course I only want my app users to see the -port flag.

Workaround

I'd like to share my workaround.

• The flag.FlagSet struct has a Usage field, which is just a func() that's called to print the usage
• When an app defines flag like var port = flag.Int(...) and calls flag.Parse, the global, exported variable flag.CommandLine is used, which is such a flag.FlagSet
• => So we overwrite flag.CommandLine.Usage with our own func()!

We can copy everything in flag's func (f *FlagSet) PrintDefaults(), but add these lines:

        // Skip printing usage info for Fiber's flags
        if f.Name == "prefork" || f.Name == "child" {
            return
        }

Some other code has to be copied from the stdlib as well to make the function compile, like stringValue, but it's not too much and in the end it works fine.

Link to example commit that contains all required changes for this workaround: https://github.com/Deflix-tv/go-stremio/commit/a57d3d418f5adf01a8854121245066692e97ab59

Now of course this is far from optimal as for example if we do this in a library that uses Fiber and our library user defines his own flags, he could define -prefork and wouldn't see the usage being printed!

That's why this is just a workaround.

[Fenny]

The problem is if we don't define the -prefork flag and an end-user calls flag.Parse() it will throw the following error:
flag provided but not defined: -prefork

The flag -prefork is reserved in Fiber, this is something we won't change anytime soon. I'm sorry to disappoint you but I think you're stuck with your workaround for now.

[Fenny]

I'm closing this issue for now, feel free to re-open if you have any questions/suggestions or alternatives regarding this topic ❤️

[doingodswork]

Hm that was quickly closed, despite it affecting everyone who uses Fiber, and without much thought about or engaging in discussion around alternative ways 🙁.

My issue is not with -prefork being reserved, it's about it _appearing in every app's printed usage despite the app's developer not having added the flag_. This will obviously confuse many users of those apps.

But I like that you're very responsive to issues, active in the community (Fiber Discord) and I like the momentum that currently exists around Fiber. So I'd like to help you 🙂.

First thing is that I misunderstood the purpose of your usage of the flags package. I thought you had in the past used os.Args and were now using the flags package to read the flags. But you're still reading the flag via os.Args, so you're using the flag package 1) to let end users know they can pass the -prefork flag when starting the app, and 2) to prevent users to define their own -prefork and -child flags so they don't clash (i.e. a Fiber user defining one of those flag with a different meaning, then the end user starting with that CLI argument, and Fiber then not having a way to know what the end user actually wanted to do), right?

I think this can be more refined.

If I understand the code correctly (I didn't dive too deep into it), -prefork is _only_ used for the initial start of the app. And -child is _only_ used for child processes.

So why don't you:

1. Let Fiber users define the -prefork flag entirely by themselves and let the info about it being passed to you via fiber.Settings.Prefork. This setting already exists and I don't see any reason why you'd also want to use (and thus _hijack_) flags here. Users can then decide to use whatever CLI argument library they want, define a prefork flag the way they want (--prefork with two slashes, -p, ...), decide on their own whether they want to expose that setting to end users at all, and have full control over this.
2. Remove -child from the usage message as it shouldn't be passed to the initial app process anyways

In fact, I just forked Fiber and implemented that and it works as expected.

I'll create a PR 🙂

[Fenny]

Hey @doingodswork, I appreciate your contribution. I didn't mean to end the discussion when I closed the issue, I just thought we solved the issue and it's oke to re-open when you have _any questions/suggestions or alternatives_ solutions 👌

I try to spent many hours of my free time to make Fiber a great framework for others to use free of charge, so I'm more than happy to have discussions and see that people contribute to the project 🙏

Regarding the subject, It's been a while but I remember that we moved from arguments to flags because if an end-user used flag.Parse() it would throw an error ( if child procs exist ) stating that some flags are not defined ( -prefork -prefork-child ).

I like the idea to remove -prefork as a "reserved" flag to enable prefork, I agree that it's up to the user how it wants to pass env or flags to the process. The user could just enable preforking within the application settings.

Thank you for contributing 👍

[Fenny]

PR got merged, it will be shipped with the next tag 👍

[doingodswork]

Awesome! Thanks a lot!

[Fenny]

@doingodswork shipped with v1.12.5 👍