Evaluating an attribute not in the request's context should return a zero value, not an error

[ZackButcher]

Today if you try to call the mixer without specifying all of the attributes required to satisfy all expression evaluated at runtime the call will fail. This is because we return errors when we evaluate expressions that contain attributes that are not in the request's attribute bag. Rather than failing the evaluation, we should return a zero value instead so that we can always continue with reasonable values at runtime.

@mandarjog points out that some of the cases aren't straightforward: e.g. what's the zero value of an email?

[kyessenov]

I think it would wise to never throw errors in expressions (think mysql, jquery, etc). It's up to you to assign values to undefined expressions such as 1/0 or empty email address.

[mandarjog]

I agree that a user should never see a 500 because something could not be evaluated.

We could start with -- in case of an error Expression gets its baseTypes' default value. We could let the user specify an alternate value, in case of an evaluation failure.

[ldemailly]

I agree the | "unknown" needed all over isn't right

[RoeyMaor]

The problem is that for many VALUE_TYPEs default value literals cannot be created. perhaps the approach should be the same as with the "ip" function defined here: https://github.com/istio/mixer/blob/535eb564667cef6aed334cb4f5e967a104768387/pkg/expr/func.go that allows creating IP_ADDRESS literals from strings. Maybe part of the solution is to define similar functions for the other VALUE_TYPEs that allow to create literals, that way we don't enforce "weird" zero values on the user and give him the option to use whatever values it wishes, and he might even choose to use those not as default values, but as the actual values for testing. Another discussion is whether These "new" string->value_type_literal functions should crash when the string given as input doesn't match their logical type format ("istio.io" as email, "42" as a timestamp, etc) regardless of this, indeed when no default value is stated, there should be a warning rather than an error

[mandarjog]

Forcing the user to give default value for an expression is similar to forcing a user to catch an exception. It is explicit and there are no surprises.

ip or other type conversion functions let you specify a typed default values, including create a typed literal. We are adding all those shortly, however that does not address the usability issue.

If that is turned into a warning, the

re: Crashing, Mixer should never crash because of user input. However evaluation may fail. Typing error of the kind you suggest are caught during validation stage. These do not result in runtime errors.

We can add missingAttributePolicy as separate settings for {report, check} X {predicate, mapping}. I wonder how UX would be when things don't work as expected.

[ldemailly]

IMO it's not similar because the compiler doesn't tell you you should catch said exception the default/least surprise (and smallest config input/least inconvenience to authors) is to have 1 or 2 level of default (per type and optional per attribute) and only throw if specifically asked to throw vs the other way around (ie a throw_if_nil(attribute.foo))

[ldemailly]

copied to https://github.com/istio/istio/issues/3598 not sure if you want to keep issues open here ? shouldn't they all get moved?