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

text/template/parse: remove truncated error context or add an template.Option #27930

Closed
draeron opened this issue Sep 28, 2018 · 8 comments
Closed
Labels
FrozenDueToAge NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one.
Milestone

Comments

@draeron
Copy link
Contributor

draeron commented Sep 28, 2018

What version of Go are you using (go version)?

go1.11 windows/amd64

Problem

When an error is thrown because a template is missing a value ("missingkey=error"), the error sent will truncate the context upon which the field is evaluated.

executing "someTemplateName" at <.k8s.clusters.someCl...>: map has no entry for key "someField"

Sample code to demontrate : https://play.golang.org/p/h3_fgkbUo_S

This is where the string gets truncated :

if len(context) > 20 {

Why is that a problem

I'm pretty sure this has been met by many users using tools that use the text/template engine (hello helm users). Once the context is deep enough, it become excitedly difficult to understand where in the data tree something is missing. If the field is dynamically accessed, say through a variable, it become pretty much impossible to know what exactly is wrong and how to fix the template.

My particular use case is that i'm trying to list all accessed values in a template, since it is impossible to access the internal structure of the parse.Tree, i though i could execute a template, catch missing value errors and construct a list through error messages.

Proposal for a solution

There are multiple solutions

  • add more specific error (parse.ValueNotFound ?) which give detailed informations which include the context
  • modify func ErrorContext to remove truncation
  • enable some option to keep the behavior backward compatible
@adamdecaf
Copy link
Contributor

Removing the truncating is likely fine. Can you submit a CL and verify? (On those nested helm charts.)

@draeron
Copy link
Contributor Author

draeron commented Oct 2, 2018

i'm not sure what you mean by CL, you mean a merge request?

@ianlancetaylor
Copy link
Contributor

Yes, a CL (a Change List) is the same as GitHub pull request. For more on contributing see https://golang.org/doc/contribute.html. Thanks.

@dmitshur dmitshur added the NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. label Oct 3, 2018
@dmitshur dmitshur added this to the Go1.12 milestone Oct 3, 2018
@draeron
Copy link
Contributor Author

draeron commented Oct 9, 2018

just bumping the issue, my request for the CLA got stuck in my employer's legal black hole

@draeron
Copy link
Contributor Author

draeron commented Oct 15, 2018

@gopherbot
Copy link

Change https://golang.org/cl/142217 mentions this issue: text/template: removed truncation of context in error message

@robpike
Copy link
Contributor

robpike commented Oct 15, 2018

This is not a compelling example. The problem is clearly visible in the error message. Even this variant:
https://play.golang.org/p/7VcbyRpWxNt
shows all you need to know.

Let's not decide what to do here until we have actual demonstration of an issue. It's possible there's something that needs fixing, but an example that illustrates it needs to be shown first.

@draeron
Copy link
Contributor Author

draeron commented Oct 16, 2018

How about this example ?

https://play.golang.org/p/pydq1PHVmQr

it become hard to figure out that the map value is missing the key is with .some.super.deep.acc... as context in the message.

@golang golang locked and limited conversation to collaborators Oct 23, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
FrozenDueToAge NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one.
Projects
None yet
Development

No branches or pull requests

6 participants