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

Add a try (as in try/catch) template func #9737

Open
bep opened this issue Apr 1, 2022 · 13 comments
Open

Add a try (as in try/catch) template func #9737

bep opened this issue Apr 1, 2022 · 13 comments
Assignees
Labels
Milestone

Comments

@bep
Copy link
Member

bep commented Apr 1, 2022

So, what I propose here is doable (I think), but we should think hard and long before we decide if and how.

I was reminded of this in

https://discourse.gohugo.io/t/catch-exception-for-transform-unmarshal-csv/37965/10

We currently have a special case for resources.GetRemote which, when it fails, returns a "Resource proxy" with .Err set.

My idea would be to do something like this:

{{ $g := "hello = \"Hello Hugo\"" | transform.Unmarshal | try }}
{{ with $g.Err }}
  // print error (or something)
{{ else }}
  {{ $resource := $g.Value }}
{{ end }}
@bep bep added the Proposal label Apr 1, 2022
@bep bep added this to the v0.97.0 milestone Apr 1, 2022
@bep bep self-assigned this Apr 1, 2022
@davidsneighbour
Copy link
Contributor

Would that be for any function or only for functions that return error codes/information (like getJson, getCSV, etc)

@marshall007
Copy link
Contributor

It might be nicer if we could introduce a proper keyword here and make it behave more like range or with:

{{ try $value, $err := "hello = \"Hello Hugo\"" | transform.Unmarshal }}
  // $err is nil
{{ else }}
  // $val is nil, $err is not nil
{{ end }}

This ensures the value remains intact and does not have to be unwrapped in cases where you just want to swallow the error and do nothing. In other words, all the following result in the same output:

{{ try $value, $err := partial "example" . }}
  {{ $value }}
{{ else }}
  // do nothing
{{ end }}

// catch is optional
{{ try $value := partial "example" . }}
  {{ $value }}
{{ end }}

// shorthand
{{ try partial "example" . }}

@bep
Copy link
Member Author

bep commented Apr 2, 2022

Would that be for any function or only for functions that return error codes/information (like getJson, getCSV, etc)

Any.

@bep bep modified the milestones: v0.97.0, v0.98.0 Apr 13, 2022
@bep bep mentioned this issue Apr 19, 2022
bep added a commit to bep/hugo that referenced this issue Apr 19, 2022
@bep
Copy link
Member Author

bep commented Apr 19, 2022

I have created a quick prototype in #9797

I do agree that a "proper keyword" solution would be (/cc @marshall007) great, but unless someone comes up with something brilliant, I'm afraid that needs to be formulated into a proposal for the Go issue tracker.

My PR is simple enough, though, is something that I would probably want:

The only current testcase I have is:

{{ $g :=  try ("hello = \"Hello Hugo\"" | transform.Unmarshal)   }}
{{ with $g.Err }}
Err1: {{ . }}
{{ else }}
Value1: {{ $g.Value.hello | safeHTML }}|
{{ end }}
{{ $g :=  try ("hello != \"Hello Hugo\"" | transform.Unmarshal)   }}
{{ with $g.Err }}
Err2: {{ . | safeHTML }}
{{ else }}
Value2: {{ $g.Value.hello | safeHTML }}|
{{ end }}

Which prints:

Value1: Hello Hugo|
Err2: template: index.html:7:52: executing \"index.html\" at <transform.Unmarshal>: error calling Unmarshal: unmarshal failed: toml: expected character =

The above should be general enough to catch any error (e.g. also uncaught nilpointers etc.), and you get the original error (with line/col info).

Would the above be ... good enough?

@marshall007
Copy link
Contributor

@bep ah, I assumed there would be some API to add keywords/rewrites similar to how you can expose custom functions.

Given that's not the case, perhaps we can avoid introducing more API surface area by leveraging the Resource type here? It already has a similar API in that it exposes .Err and .Content.

We could have resources.Unmarshal act as shorthand for "hello = \"Hello Hugo\"" | resources.FromString "<md5>.toml" and the resulting object contain a .Value property with the unmarshal'd content. We could at the same time, take an extra step and do this automatically for well-known content types (JSON, YAML, TOML).

This would be handy for a few reasons:

  1. no new function API surface area to learn/adopt
  2. automatically parse well-known content types when using Get and GetRemote (avoid extra .Content | transform.Unmarshal which also has to be handled)
  3. .Err value can represent HTTP errors, parser errors, and template exec errors
  4. original .Content can be exposed alongside .Value when a parser error occurs

So this would look like:

{{ with  "hello = \"Hello Hugo\"" | resources.Unmarshal }}
{{ if .Err }}
Err1: {{ . }}
{{ else }}
Value1: {{ .Value.hello | safeHTML }}|
{{ end }}

If the data is stored in a file on disk:

{{ with resources.Get "assets/data.toml" }}
{{ if .Err }}
Err1: {{ . }}
{{ else }}
Value1: {{ .Value.hello | safeHTML }}|
{{ end }}

@bep
Copy link
Member Author

bep commented Apr 19, 2022

@marshall007 what you propose is basically what we have in resources.GetRemote -- which is what this issue was meant to make into a general thing. Your proposal is incredibly expensive/intrusive if you want to 1) Make it work with everything (transform.Unmarshal was just a random example) and 2) Allow it to be backwards compatible.

@marshall007
Copy link
Contributor

@bep that's fair but use cases outside of resources.Get/GetRemote and/or unmarshal seem sparse. HTTP and parser errors are about the only thing you'd ever care to print/introspect in a template. All other functions (that I can think of) have very predictable behavior that is the result of user error.

Consider the inverse in having default adopt the semantics of your proposed try function whereby errorf "..." | default "foo" returns "foo" might actually be more useful in the common case. Note the original ask was just that the user wanted to "handle the error" not necessarily "render the error in the template".

@bep
Copy link
Member Author

bep commented Apr 20, 2022

use cases outside of resources.Get/GetRemote and/or unmarshal seem sparse.

You mean your use cases outside these. Any func/method that may fail on bad input data is a candidate. One example would be all the image processing methods/filters (I have had situations with PNG with malformed headers where I wish I could just default to a placeholder).

Note the original ask was just that the user wanted to "handle the error" not necessarily "render the error in the template".

I want to both handle the error and also sometimes render the error (providing as much detail as possible so I can find it).

errorf "..." | default "foo"

I have no idea how the above would work/solve this issue.

@jmooring
Copy link
Member

I just ran into case where this would be helpful.

{{ $w := "1" }}
{{ $wAsInteger := int $w }}

Except that I have no control over $w -- it could be "foo".

@bep bep modified the milestones: v0.98.0, v0.99.0 Apr 28, 2022
@bep bep modified the milestones: v0.99.0, v0.100.0 May 24, 2022
@bep bep modified the milestones: v0.100.0, v0.101.0 May 31, 2022
@bep bep modified the milestones: v0.101.0, v0.102.0 Jun 16, 2022
@fekete-robert
Copy link
Contributor

Would this be able to provide a fallback for circular loops in shortcodes, like the problem described in https://discourse.gohugo.io/t/you-may-have-a-circular-loop-in-a-shortcode-problem/ ?

@bep
Copy link
Member Author

bep commented Jun 22, 2022

@fekete-robert yes, that should be possible. But if it's a real circular loop I would recommend to ... fix the underlying issue.

@jmooring I have forgotten about this issue. #9797 should work ...

@fekete-robert
Copy link
Contributor

@bep Thanks! I'd say my issue is an implicitly triggered circular loop caused by a valid use case (from me as the user's perspective) :)

@bep bep modified the milestones: v0.102.0, v0.103.0 Aug 28, 2022
@bep bep removed this from the v0.103.0 milestone Sep 15, 2022
@bep bep added this to the v0.111.0 milestone Jan 26, 2023
@bep bep modified the milestones: v0.111.0, v0.112.0 Feb 15, 2023
@bep bep modified the milestones: v0.112.0, v0.113.0 Apr 15, 2023
@bep bep modified the milestones: v0.113.0, v0.115.0 Jun 13, 2023
@bep bep modified the milestones: v0.115.0, v0.116.0 Jun 30, 2023
@bep bep modified the milestones: v0.116.0, v0.117.0 Aug 1, 2023
@bep bep modified the milestones: v0.117.0, v0.118.0 Aug 30, 2023
@bep bep modified the milestones: v0.118.0, v0.119.0 Sep 15, 2023
@bep bep modified the milestones: v0.119.0, v0.120.0 Oct 5, 2023
@bep bep modified the milestones: v0.120.0, v0.121.0 Oct 31, 2023
@bep bep modified the milestones: v0.121.0, v0.122.0 Dec 6, 2023
@bep bep modified the milestones: v0.122.0, v0.123.0, v0.124.0 Jan 27, 2024
@bep bep modified the milestones: v0.124.0, v0.125.0 Mar 4, 2024
@bep bep modified the milestones: v0.125.0, Unscheduled Oct 23, 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

6 participants