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

Seeing a panic when using predefined rules #148

Closed
doriable opened this issue Sep 30, 2024 · 0 comments · Fixed by #149
Closed

Seeing a panic when using predefined rules #148

doriable opened this issue Sep 30, 2024 · 0 comments · Fixed by #149
Assignees
Labels
Bug Something isn't working

Comments

@doriable
Copy link
Member

Description

When setting a value for a field with predefined rules, we get a panic:

$ go run main.go
panic: interface conversion: *types.Err is not traits.Lister: missing method Add

Steps to Reproduce

This is an example repository with a repro: https://github.com/doriable/pv-go-bug

Basically this just creates a message and attempts to set the value. Included is also a message that does not use predefined rules to compare the behaviour.

Expected Behavior

We should be able to set the value safely and run validation.

Actual Behavior

We are seeing a panic instead.

Environment

  • Operating System: macOS
  • Version: Sonoma 14.6
  • Compiler/Toolchain: go version go1.23.1 darwin/arm64
  • Protobuf Compiler & Version: buf 1.43.0
  • Protovalidate Version: pvgo 0.7.0, protovalidate 0.8.0
@doriable doriable added the Bug Something isn't working label Sep 30, 2024
rodaine added a commit that referenced this issue Sep 30, 2024
Using a repeated field as a predefined rule value results in a panic, as
CEL cannot convert a `protoreflect.List` value into a valid CEL type.
This results in an error value standing in for the list's value in the
expression. When attempting to optimize the expression via computing
residuals, a type assertion without the guard boolean (i.e., `typed :=
unknown.(T)` instead of `typed, ok := unknown.(T)`) triggers the panic
on this non-list error value.

This patch adds a new helper to the celext package to compute an
appropriate value for any `protoreflect.Value` given its field
descriptor.

Fixes #148
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Bug Something isn't working
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants