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

ParseWithOptions not returning error from custom function #268

Closed
flohoss opened this issue Jun 5, 2023 · 5 comments
Closed

ParseWithOptions not returning error from custom function #268

flohoss opened this issue Jun 5, 2023 · 5 comments
Labels

Comments

@flohoss
Copy link

flohoss commented Jun 5, 2023

Hi,

i am trying to work this out for a while now. The main goal is to parse a shoutrrr url from envs.

I could just create a sender with a string that has been parsed and check for error there. But i would like to use the FuncMap for custom parsing. What am i doing wrong?

  • Is the required param ignored even though i am setting the env to ""?
  • Why is the error that exists in line 21 not recognized in line 34?

The code is based on the the example

https://play.golang.com/p/4h5IqZLB0Hs

package main

import (
	"log"
	"os"
	"reflect"

	"github.com/caarlos0/env/v8"
	"github.com/containrrr/shoutrrr"
	"github.com/containrrr/shoutrrr/pkg/router"
)

type config struct {
	// is required ignored, even though the pointer is nil?
	Sender *router.ServiceRouter `env:"NOTIFICATION_URL,required"`
}

func main() {
	os.Setenv("NOTIFICATION_URL", "")

	_, err := shoutrrr.CreateSender(os.Getenv("NOTIFICATION_URL"))
	if err != nil {
		log.Println(err)
	}

	cfg := config{}
	err = env.ParseWithOptions(&cfg, env.Options{
		OnSet: func(tag string, value interface{}, isDefault bool) {
			log.Printf("Set %s to '%s' (default? %v)\n", tag, value, isDefault)
		},
		FuncMap: map[reflect.Type]env.ParserFunc{
			reflect.TypeOf(router.ServiceRouter{}): func(v string) (interface{}, error) {
				// Same as in line 21
				return shoutrrr.CreateSender(v)
			},
		},
	})
	if err != nil {
		log.Fatal(err)
	}
}

When run with debugger and NOTIFICATION_URL not set, the error returned from ParseWithOptions() is not nil:
error(github.com/caarlos0/env/v8.AggregateError) {Errors: []error len: 1, cap: 1, [*(*error)(0x1400017c320)]}
But as soon as the env is set, but empty, no error is returned...

@flohoss flohoss changed the title ParseWithOptions not returning err from Shoutrrr.CreateSender ParseWithOptions not returning error from Shoutrrr.CreateSender but AggregateError Jun 5, 2023
@flohoss flohoss changed the title ParseWithOptions not returning error from Shoutrrr.CreateSender but AggregateError ParseWithOptions not returning error but AggregateError Jun 5, 2023
@flohoss flohoss changed the title ParseWithOptions not returning error but AggregateError ParseWithOptions not returning error from custom function Jun 5, 2023
@caarlos0 caarlos0 added the bug label Jun 7, 2023
@caarlos0
Copy link
Owner

caarlos0 commented Jun 8, 2023

have you tried this? https://github.com/caarlos0/env#not-empty-fields

@caarlos0
Copy link
Owner

caarlos0 commented Jun 8, 2023

ahh I see what you mean, regardless of that, the error from the custom parser does not bubble up.

nice, thanks for reporting, will take a look.

fwiw, minimal reproducible: https://play.golang.com/p/ijP_1MxhRp7

caarlos0 added a commit that referenced this issue Jun 8, 2023
Signed-off-by: Carlos Alexandro Becker <caarlos0@users.noreply.github.com>
@akutuev
Copy link
Contributor

akutuev commented Jun 8, 2023

@caarlos0, should I take a look if I find time?

@caarlos0
Copy link
Owner

caarlos0 commented Jun 9, 2023

see #269

@caarlos0
Copy link
Owner

ok, after looking into this again, this is the expected behavior:

  • required means that the env needs to be set, not empty
  • if env is set but empty, the field will have its default value

if you want this to fail, you can set notEmpty instead of required.

@caarlos0 caarlos0 closed this as not planned Won't fix, can't repro, duplicate, stale Jun 15, 2023
BorzdeG pushed a commit to BorzdeG/env that referenced this issue Aug 24, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

3 participants