helm-playground icon indicating copy to clipboard operation
helm-playground copied to clipboard

Required and fail do nothing

Open Cenness opened this issue 1 year ago • 6 comments

Template

{{ fail "message" }}
name: {{ .Values.unset | required "unset messsage" }}

and no values produce following output:


name: <no value>

Cenness avatar Sep 25 '24 14:09 Cenness

Helm-playground intentionally does not throw an error for these, because an error would cause the entire template to not be rendered. Meaning, if you wanted to debug a template in helm-playground that has required or fail, you would very often need to comment them out in order to get it rendered initially, which is annoying.

Code:

https://github.com/shipmight/helm-playground/blob/bec0a8502a7b274b5f24791317d0872fdabf2a8a/go/lib/lib.go#L131-L135

https://github.com/shipmight/helm-playground/blob/bec0a8502a7b274b5f24791317d0872fdabf2a8a/go/lib/lib.go#L138-L142

But I'm open to suggestions on how it should behave.

Would it make more sense to print an error text into the template?

codeclown avatar Jan 29 '25 10:01 codeclown

I've expected playground to produce message passed to those functions. And not produce anything else.

Cenness avatar Jan 29 '25 11:01 Cenness

Helm-playground intentionally does not throw an error for these, because an error would cause the entire template to not be rendered. Meaning, if you wanted to debug a template in helm-playground that has required or fail, you would very often need to comment them out in order to get it rendered initially, which is annoying.

Code:

helm-playground/go/lib/lib.go

Lines 131 to 135 in bec0a85

// If the template contains required, we don't want to return an error which // would prevent previewing the entire template. Simply pass the value through. funcMap["required"] = func(warn string, val interface{}) (interface{}, error) { return val, nil } helm-playground/go/lib/lib.go

Lines 138 to 142 in bec0a85

// If the template contains fail, we don't want to return an error which // would prevent previewing the entire template. Return an empty string. funcMap["fail"] = func(val interface{}) (interface{}, error) { return "", nil } But I'm open to suggestions on how it should behave.

Would it make more sense to print an error text into the template?

This is a poor decision because, in debugging scenarios, fail is intentionally used to return values. Consider the following construct:

{{ $some_var | toString | fail }}

This approach is quite common, so a templating failure is not only correct—mirroring Helm's behavior—but also intentional. How can you properly debug if you distort the logic in such an unconventional way?

JuryA avatar Mar 18 '25 11:03 JuryA

Small example why this behavior is bad (it's simplified):

{{- define "fn" -}}

{{- /*Intentional check if `.INPUT` is a type of 'map' because we need to return a non-string value by mutating a mutable type—in our case, a `map`.*/ -}}
{{- if ($return := index . "INPUT") | kindIs "map" -}}
  {{- /* ... some logic continues ... */ -}}
{{- else -}}
  {{- /* HERE WE NEED TO FAIL! YOUR TWEAK FAILS QUICKLY HERE! */ -}}
  {{- fail "INPUT must be MAP!" -}}
{{- end -}}

{{- end -}}

{{- /* In this setup, if `$input` is not a mutable `map`, it incorrectly does **NOT** fail. */ -}}
{{- $input := (list) -}}

{{- $_ := include "fn" (dict "INPUT" $input) -}}

I expect it to fail—it's your responsibility as a developer to catch all potential errors. If you're 'annoyed' by this correct behavior, crippling the fail functionality is the worst possible solution and a highly dangerous hack. Simply put, the behavior of your tool is not the same as Helm’s.

JuryA avatar Mar 18 '25 11:03 JuryA

To be honest, I can certainly see the usefulness of such behavior and would absolutely take advantage of it. It's a smart solution, no doubt! I don’t consider it a bad idea at all—quite the opposite. However, I would personally make it opt-in; if I want it, I’ll enable it myself. Otherwise, it leads to severe inconsistency. In this case, consistency always takes higher priority for me, even if it’s potentially flawed.

I don’t want to sound like an ungrateful complainer—I fully understand that this amazing playground was created primarily to serve its author’s needs. Please take this as constructive feedback for potential improvements. That aside, it’s an absolutely fantastic tool! I use it almost daily for quick tests. I just happened to spend the last few months working on other things, and the very first issue I ran into upon returning was debugging your 'modified' fail behavior... 😸

JuryA avatar Mar 18 '25 11:03 JuryA

Glad to hear the tool is useful!

Arguments make sense. I'm willing to review a PR which implements a more expected behaviour.

codeclown avatar Mar 18 '25 15:03 codeclown