Skip to content

Consider try / catch functionality #415

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

Open
sparkprime opened this issue Dec 1, 2017 · 8 comments
Open

Consider try / catch functionality #415

sparkprime opened this issue Dec 1, 2017 · 8 comments

Comments

@sparkprime
Copy link
Collaborator

sparkprime commented Dec 1, 2017

Currently error "foo" is like throwing an exception, but there is no way to catch the exception.

Motivations for "catch":

  1. Routines that loop over data and trigger errors for particular items don't have useful stack traces because the line of code in the stack trace doesn't tell you e.g. which array index caused the problem. This is particularly important in custom manifestation functions where we typically iterate over the tree forcing fields.
  2. It is currently not possible to write test cases in Jsonnet that check that the correct error is raised.
  3. It is not possible to detect using reflection that a field requires overriding, because we typically represent that with f: error "Must overide f" see std.mergePatch can't merge over error values #414.

Caveats:

  • Should it be possible to catch "runtime errors" like 1/0 or [1,2,3][10]? If not, how do we distinguish?
  • Should it be possible to catch major errors like exceeding stack size? If not, how do we distinguish?
  • It is clearly not possible to catch non-termination (halting problem).
  • Should error values be cached in thunks?
@sparkprime
Copy link
Collaborator Author

@mikedanese and @yugui for unit testing

@sparkprime
Copy link
Collaborator Author

sparkprime commented Dec 1, 2017

It's notable that all 3 motivations currently have workarounds that require only limited try / catch functionality:

  1. A construct that allows augmenting an error with extra contextual information, but always propagates the error
  2. A construct that requires its subexpression to return an error, and returns the message in that error. This would not allow "branching" on whether the error was present or not.
  3. Use of a special first class value that raises an error at manifestation time, but can be passed around / inspected as a value. A simple example is function() null which cannot be manifested.

@gaurav517
Copy link

gaurav517 commented Oct 20, 2020

try/catch could be useful in a case like following:

namespace:: if(std.extVar('namespace')) then std.extVar('namespace') else 'default',

Currently it fails with RUNTIME ERROR: Undefined external variable: namespace if namespace is not passed. With try/catch, we could catch the error and return default.
On this note, is there any workaround/better way to check whether an external variable is provided?

Thanks,

@sbarzowski
Copy link
Collaborator

sbarzowski commented Oct 20, 2020

It was discussed the possibility of optional extVars in #304. The decision was that they are a bad idea for discoverability reasons. It's likely to encourage harmful patterns like depending on implicit set of extVars in libraries.

@brettviren
Copy link
Contributor

Hi @gaurav517

On this note, is there any workaround/better way to check whether an external variable is provided?

If you don't mind me suggesting a solution to a slightly different problem which exists if one switches from extVar to TLAs:

Instead of

{
    foo: std.extVar("foo")
}

Refactor to be:

function (foo = 42) {
    foo: foo
}

Exercising like:

$ jsonnet -A foo=21 -e 'function (foo = 42) { foo:foo }'
{
   "foo": "21"
}

Or, to get a number instead of a string

$ jsonnet --tla-code foo=21 -e 'function (foo = 42) { foo:foo }'
{
   "foo": 21
}

Alternatively, you could set the TLA in the function definition as foo = null and test in the body of the function (std.type(foo) == "null") to see if it was unset. Or you can simply not provide a default value if you want the caller to be forced to provide one.

I had to do a bit of refatoring when I switched from extVars to TLAs but in the end it was well worth it.

@gaurav517
Copy link

Thanks. switching to TLA would need wrapping in function().. but in that case, how can we render from a function inside function() block.
For example, I have library libsonnet (my-lib.libsonnet):

{
  getManifests():: {
    # getting a, b, c, d from extVar
    deployment: {..},
    service: {..}
  }
}

Now I have a jsonnet that uses that library:

local lib = import 'my-lib.libsonnet';
lib.getManifests()

and it works. But if we wrap it with function():

local lib = import 'my-lib.libsonnet';
function(a, b, c, d){
  lib.getManifests(a,b,c,d)
}

then it won't work, because its expecting key:value inside function(){}.
How can we get around this? Thanks.

@sbarzowski
Copy link
Collaborator

then it won't work, because its expecting key:value inside function(){}.

You don't need the {}. They're part of object literal syntax, not function syntax and that's why they need to be followed by key:value – it's an object definition. This is a pretty common confusion for people coming from curly brace languages like C, Java or Go.

So in your case it will be just:

local lib = import 'my-lib.libsonnet';
function(a, b, c, d) lib.getManifests(a,b,c,d)

@gaurav517
Copy link

Thanks. yeah, getting used to syntax slowly. :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants