Can't partly omit nested messages using --dig-manually #554
Comments
|
Thanks for the report. I think this can be fixed easily. We can define a new error such like If you are interested, would you like to fix this bug? |
|
Yeah, sounds good, if it will make the fix come out faster I'm all for it. I've glanced at the code, so can I potentially handle it by simply using |
|
I added new error to prompt/prompt.go var (
ErrAbort = errors.New("abort")
ErrSkip = errors.New("skip")
)changed ErrAbort to ErrSkip in if r.skipMessage(f) {
return nil, prompt.ErrSkip
}and added new check in err := r.resolveField(f)
if errors.Is(err, prompt.ErrSkip) {
continue
}
if errors.Is(err, prompt.ErrAbort) {
return r.msg, nil
}and now skip in However this change broke something. When the program tries to parse not nil response from the gRPC API it crashes with (when the response is nil everything is OK): Any ideas on what may have caused it and how it can be fixed? I'm, honestly, quite perplexed as to why a small change in behaviour of message creator caused |
|
Umm... |
|
Ok, here are the changes to the functions and vars: fill/proto/interactive_filler.go func (r *resolver) resolve() (*dynamic.Message, error) {
selectedOneof := make(map[string]interface{})
for _, f := range r.m.GetFields() {
if isOneOfField := f.GetOneOf() != nil; isOneOfField {
fqn := f.GetOneOf().GetFullyQualifiedName()
if _, selected := selectedOneof[fqn]; selected {
// Skip if one of choices is already selected.
continue
}
selectedOneof[fqn] = nil
if err := r.resolveOneof(f.GetOneOf()); err != nil {
return nil, err
}
continue
}
err := r.resolveField(f)
if errors.Is(err, prompt.ErrSkip) {
continue
}
if errors.Is(err, prompt.ErrAbort) {
return r.msg, nil
}
if err != nil {
return nil, err
}
}
return r.msg, nil
}
func (r *resolver) resolveField(f *desc.FieldDescriptor) error {
resolve := func(f *desc.FieldDescriptor) (interface{}, error) {
var converter func(string) (interface{}, error)
switch t := f.GetType(); t {
case descriptorpb.FieldDescriptorProto_TYPE_MESSAGE:
if r.skipMessage(f) {
return nil, prompt.ErrSkip
}
msgr := newResolver(
r.prompt,
r.prefixFormat,
r.color.NextVal(),
dynamic.NewMessage(f.GetMessageType()),
append(r.ancestors, f.GetName()),
r.repeated || f.IsRepeated(),
r.opts,
)
return msgr.resolve()
case descriptorpb.FieldDescriptorProto_TYPE_ENUM:
return r.resolveEnum(r.makePrefix(f), f.GetEnumType())
case descriptorpb.FieldDescriptorProto_TYPE_DOUBLE:
converter = func(v string) (interface{}, error) { return strconv.ParseFloat(v, 64) }
case descriptorpb.FieldDescriptorProto_TYPE_FLOAT:
converter = func(v string) (interface{}, error) {
f, err := strconv.ParseFloat(v, 32)
return float32(f), err
}
case descriptorpb.FieldDescriptorProto_TYPE_INT64,
descriptorpb.FieldDescriptorProto_TYPE_SFIXED64,
descriptorpb.FieldDescriptorProto_TYPE_SINT64:
converter = func(v string) (interface{}, error) { return strconv.ParseInt(v, 10, 64) }
case descriptorpb.FieldDescriptorProto_TYPE_UINT64,
descriptorpb.FieldDescriptorProto_TYPE_FIXED64:
converter = func(v string) (interface{}, error) { return strconv.ParseUint(v, 10, 64) }
case descriptorpb.FieldDescriptorProto_TYPE_INT32,
descriptorpb.FieldDescriptorProto_TYPE_SFIXED32,
descriptorpb.FieldDescriptorProto_TYPE_SINT32:
converter = func(v string) (interface{}, error) {
i, err := strconv.ParseInt(v, 10, 32)
return int32(i), err
}
case descriptorpb.FieldDescriptorProto_TYPE_UINT32,
descriptorpb.FieldDescriptorProto_TYPE_FIXED32:
converter = func(v string) (interface{}, error) {
u, err := strconv.ParseUint(v, 10, 32)
return uint32(u), err
}
case descriptorpb.FieldDescriptorProto_TYPE_BOOL:
converter = func(v string) (interface{}, error) { return strconv.ParseBool(v) }
case descriptorpb.FieldDescriptorProto_TYPE_STRING:
converter = func(v string) (interface{}, error) { return v, nil }
// Use strconv.Unquote to interpret byte literals and Unicode literals.
// For example, a user inputs `\x6f\x67\x69\x73\x6f`,
// His expects "ogiso" in string, but backslashes in the input are not interpreted as an escape sequence.
// So, we need to call strconv.Unquote to interpret backslashes as an escape sequence.
case descriptorpb.FieldDescriptorProto_TYPE_BYTES:
converter = func(v string) (interface{}, error) {
if r.opts.BytesFromFile {
b, err := ioutil.ReadFile(v)
if err == nil {
return b, nil
}
}
v, err := strconv.Unquote(`"` + v + `"`)
return []byte(v), err
}
default:
return nil, fmt.Errorf("invalid type: %s", t)
}
prefix := r.makePrefix(f)
return r.input(prefix, f, converter)
}
if !f.IsRepeated() {
v, err := resolve(f)
if err != nil {
return err
}
return r.msg.TrySetField(f, v)
}
color := r.color
for {
// Return nil to keep inputted values.
if !r.addRepeatedField(f) {
return nil
}
r.prompt.SetPrefixColor(color)
color.Next()
v, err := resolve(f)
if err == io.EOF {
// io.EOF signals the end of inputting repeated field.
// Return nil to keep inputted values.
return nil
}
if err != nil {
return err
}
if err := r.msg.TryAddRepeatedField(f, v); err != nil {
return err
}
}
}prompt/promt.go var (
ErrAbort = errors.New("abort")
ErrSkip = errors.New("skip")
)Those are the only changes I made. Just paste the fucntions and replace var block and you should have the same code as me. If you still aren't able to reproduce it, my build must be broken or something (becasue it's the google proto package that crashes). If that's the case I can create pull request. |
|
@relipocere |
|
@ktr0731 Yes, |
|
@relipocere It can be run with the following command: $ go run github.com/ktr0731/grpc-test@latest -r |
|
@ktr0731 yup, I tried and it works fine with "simple" types. Looks like it breaks when the response message includes a message in itself. Try calling this kind of api: service MyService{
rpc GetGroups(GetGroupsRequest) returns (GetGroupsResponse);
}
message GetGroupsRequest{
repeated int64 group_ids = 1;
}
message GetGroupsResponse{
repeated Group groups = 1;
}
message Group{
int64 id = 1;
string title = 2;
repeated int64 organization_ids = 3;
repeated int64 point_ids = 4;
repeated int64 region_ids = 5;
repeated int64 user_ids = 6;
} |
|
@relipocere |
|
Well, gotta check the whole call stack. It must be a rouge pointer, can't think of any other reason for it to crash at the google package level |
|
@relipocere |
|
@ktr0731 it works fine now-no more panics and |
|
Also completely unrelated, but when you're building the binary you can pass |
|
Thank you for the beneficial information. We'll add the option! |
|
Fixed in #568 |
Describe the bug
There's no way to partly omit nested messages in repl mode.
--dig-manuallyallows to omit only the currently selected field and then skips the filling of the rest of the fields. There needs to be a way to skip a field and move to the next one.To reproduce
Then try skipping organizations field completely(make it nil), but enter points.
Expected behavior
There is way to omit field organizations, but fill points so the request will look like this:
Environment
protocversion: -protocplugin version (if you are using): -Additional context
The one way to solve this issue is to add a new option to
--dig-manuallymode that will allow to skip a field without skipping the rest of them. The name can beskip oncefor example.The text was updated successfully, but these errors were encountered: