clarkmcc / cel-rust

Common Expression Language interpreter written in Rust
https://crates.io/crates/cel-interpreter
MIT License
375 stars 21 forks source link

Unresolved method/function calls panic #65

Closed alexsnaps closed 3 months ago

alexsnaps commented 3 months ago

Could be a regression because of #59 - though looks like the unwrap() on the lookup was there before. Anyways, what led me to start working on #64 is hitting that very problem invoking bytes():

panicked at interpreter/src/objects.rs:526:58:
called `Option::unwrap()` on a `None` value

Unsure what error should be reported, but panicking isn't certainly not the right thing. I'll look into it

clarkmcc commented 3 months ago

I apologize, I'm not sure I totally follow. Is this a new issue introduced by #64? I see that PR introduces an .unwrap() call that wasn't there before, but maybe that's not the unwrap that you're referring to? Can you clarify what the problem is and what expression could reproduce it? I'm happy to investigate as well.

alexsnaps commented 3 months ago

Yeah, sorry this was probably not real clear. So, yes… when calling to a function that doesn't exists (e.g. bytes("abc")), the Option.unwrap panics here, that .unwrap() was already there tho. I don't know, though I don't think so, that something else made sure the function existed prior that call before the refactoring… I guess the question becomes, what should the behavior be? UndeclaredReference based on the description there, correct?

clarkmcc commented 3 months ago

@alexsnaps thanks for the clarification, makes sense! And I agree with your proposed expected behavior