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

Unable to use Google well known types #1042

Open
respinha opened this issue May 9, 2018 · 41 comments
Open

Unable to use Google well known types #1042

respinha opened this issue May 9, 2018 · 41 comments
Labels

Comments

@respinha
Copy link

respinha commented May 9, 2018

protobuf.js version: 6.8.6

When I attempt to encode a message from a Google well known type (like google.protobuf.Struct) I get an empty field in the decoded message IF my path includes google/protobuf.
Is this a bug or do we need to provide the path in some other way. I assume the problem starts when loading the commons wrapper here.

const protobuf = require('protobufjs');

let root = new protobuf.Root();
const protoFilePath = '<path to a file which imports google.protobuf.Struct>';
const protoRoot = .............;
root.resolvePath = function (origin, target) {
  // ignore the same file
  if (target == protoFilePath) {
    return protoFilePath;
  }
  // Resolved target path for the import files
  return protoRoot + target;
};
const struct = {
  fields: {
    label: {
      struct_value: {
        fields: {
          another_label: {
            number_value: 10
          }
        }
      }
    }
  }
};
root = root.loadSync(protoFilePath);
const MyType = root.lookupType('MyType');
const msg = MyType.create({
  struct_message: struct
});

const buffer = MyType.encode(msg).finish();

console.log('Decoded message', MyType.decode(buffer));
Decoded message: Struct { fields: { label: Value {} } } }

Note: this was working with protobuf.js version 5.

@respinha
Copy link
Author

Bump: @dcodeIO, is this a bug or is there another way of using these types?

@dcodeIO
Copy link
Member

dcodeIO commented May 23, 2018

This might be due to protobuf.js implicitly converting field names from underscore_notation to camelCaseNotation since v6 (like google's js reference js impl does). You can try

root.loadSync(protoFilePath, { keepCase: true })

to retain underscore notation or change the struct contents accordingly.

@respinha
Copy link
Author

I loaded with { keepCase: true } and still the same problem.

@dcodeIO
Copy link
Member

dcodeIO commented May 23, 2018

What happens if you do:

const struct = {
  fields: {
    label: {
      kind: "struct_value",
      struct_value: {
        fields: {
          another_label: {
            kind: "number_value",
            number_value: 10
          }
        }
      }
    }
  }
};

@respinha
Copy link
Author

Still the same result...

@dcodeIO
Copy link
Member

dcodeIO commented May 23, 2018

Just edited this to use the string field names, can you try again? I believe it looks for the virtual oneof field there to determine which type of value is provided.

@respinha
Copy link
Author

Tried and still this
Struct { fields: { label: Value {} } } }

@dcodeIO
Copy link
Member

dcodeIO commented May 23, 2018

Hmm, what happens if you also do

const msg = MyType.fromObject({
  struct_message: struct
});

If that also doesn't work, can you provide the .proto definition for MyType?

@respinha
Copy link
Author

respinha commented May 23, 2018

I am using these protos: https://github.com/restorecommerce/protos. The proto definition is in io/restorecommerce/resource_base.proto (ReadRequest). It has a filter field which is of type google.protobuf.Struct. It has always worked with grpc-node which fallsback to protobufjs v5 when loading proto definitions but not if we specifically try v6.

@dcodeIO
Copy link
Member

dcodeIO commented May 23, 2018

Thanks. Seems that there is an issue with keepCase in conjunction with the struct type, because src/common.js explicitly defines it in camelCase notation, which isn't converted back when keepCase = true.

This appears to work:

var root = new protobuf.Root();
root.load("google/protobuf/struct.proto", function() {
  protobuf.parse(`syntax = "proto3"; message MyType { google.protobuf.Struct filter = 1; }`, root);
  var MyType = root.MyType;
  var myType = MyType.fromObject({
    filter: {
      fields: {
        number: {
          numberValue: 10
        },
        struct: {
          structValue: {
            fields: {
              number: {
                numberValue: 20
              }
            }
          }
        }
      }
     }
  });
  console.log(myType);
});

@respinha
Copy link
Author

Oh, interesting! Then is it a bug when passing the options to common.js?

@dcodeIO
Copy link
Member

dcodeIO commented May 23, 2018

Yes, this is a bug affecting keepCase. A proper fix could be to move field renaming out of the parser into construction of the reflection structure, in turn defining all of common.js in underscore notation.

@dcodeIO dcodeIO added the bug label May 23, 2018
@respinha
Copy link
Author

respinha commented May 23, 2018

Btw, what is the use of those custom wrappers for Google well known types? Wouldn't they be parsed correctly in the "standard" way?

@dcodeIO
Copy link
Member

dcodeIO commented May 23, 2018

Custom wrappes (from wrappers.js) provide a way to work with instances of Value, for example, in a more natural way. For example, if you provide an object that has a Value value of number to fromObject, it will automatically "wrap" it in a proper Value type. Similarly it unwraps to a single value from a Value instance in toObject. At least that's the intention.

@Jmoore1127
Copy link

Any workaround or update here? I'm blocked not being able to load protos importing google/protobuf/empty.proto. I believe this is the same issue.

@Arun-KumarH
Copy link

This is a pressing issue for us, any updates on this ?

@respinha
Copy link
Author

respinha commented Sep 3, 2018

Bump: any updates here?

@vanthome
Copy link

Will this be even fixed?

@obbardc
Copy link

obbardc commented Dec 6, 2018

An issue for me too.

@AlmogBaku
Copy link

any update?

@hollinwilkins
Copy link

This is an issue for us trying to use the standard wrappers, they are not valid JSON for other JSON parsers that handle the standard wrappers according to the JSON documentation here:
https://developers.google.com/protocol-buffers/docs/proto3#json

@duhruh
Copy link

duhruh commented May 8, 2019

I'm still experiencing this issue

Error: no such type: google.protobuf.Empty

@TAOXUY
Copy link

TAOXUY commented Jul 31, 2019

+1. Any updates?

@swan-admin
Copy link

+1. Ready to PR if needed but I need a pointer.

@lucasantarella
Copy link

+1 please fix!

@andrew8er
Copy link

Isn't this fixed with f61b4bc? If so, could you please release a new minor version?

Maybe related/duplicates of this issue:

@RMHonor
Copy link

RMHonor commented Sep 30, 2019

@andrew8er That code doesn't seem to resolve the issue.

@4nte
Copy link

4nte commented Oct 23, 2019

I'm currently patching frontend code to manually parse to JSON and back. :(

And to adjust type typescript typings for the message containing messages of type Struct, I'd create a new interface which extends the generated pb message type and replace the field that I'm manually mapping.

// This is how the generated TS type looks like
export namespace MyMessage {
    export type AsObject = {
      foo: string,
      bar: ptypes_struct_struct_pb.Struct.AsObject, // :(
    }
}
// Patch the generated types by creating a new type that extends the generated type and override field(s) which require manual mapping.
export interface MyMessageAsObject extends Omit <MyMessage.AsObject, 'bar'> {
    bar: /* Your custom type here */
}

@CatsMiaow
Copy link

The methods below also appear to be incompatible.

https://www.npmjs.com/package/google-protobuf
https://github.com/protocolbuffers/protobuf/tree/master/js

import { ListValue, Struct, Value } from 'google-protobuf/google/protobuf/struct_pb';

ListValue.fromJavaScript(['foo', 'bar']).toObject();
Struct.fromJavaScript({ foo: 'bar' }).toObject();
Value.fromJavaScript('foobar').toObject();

@Arun-KumarH
Copy link

Arun-KumarH commented May 8, 2020

protobufjs: 6.9.0
We checked the issue again with latest protobufjs version 6.9.0, but still we are not able to encode google.protobuf.Struct messages.
We are still using grpc.load which falls back to protobufjs version 5 and grpc.load is deprecated.

This issue is holding us back to use grpc\protoloader.
Any updates on this issue ?

@ssilve1989
Copy link

bump

@myleshk
Copy link

myleshk commented Jul 7, 2020

Updates?

@classLfz
Copy link

classLfz commented Jul 17, 2020

same issue here, I read the official document about struct , then I write this func to build the struct data to protobuf:

function buildGoogleStructValue (val, sub = false) {
  const typeofVal = typeof val
  const baseValueTypes = {
    number: 'numberValue',
    string: 'stringValue',
    boolean: 'boolValue'
  }
  if (Object.keys(baseValueTypes).includes(typeofVal)) {
    return {
      [baseValueTypes[typeofVal]]: val
    }
  }
  if (Array.isArray(val)) {
    const out = {
      listValue: {
        values: []
      }
    }
    val.forEach(valItem => {
      const itemVal = buildGoogleStructValue(valItem, true)
      out.listValue.values.push(itemVal)
    })
    return out
  }
  if (typeofVal === 'object') {
    const out = sub ? {
      structValue: {
        fields: {}
      }
    } : {
      fields: {}
    }
    Object.keys(val).forEach(field => {
      if (sub) {
        out.structValue.fields[field] = buildGoogleStructValue(val[field], true)
      } else {
        out.fields[field] = buildGoogleStructValue(val[field], true)
      }
    })
    return out
  }
}

proto:

message Message {
    google.protobuf.Struct struct = 1;
}

so, I can build message data like this:

const message = {
    struct: buildGoogleStructValue({
        string: '1',
        bool: true,
        number: 12,
        struct: {
            structField1: 1000
        },
        list: [1, '12']
    }
})

It's worked for me.

@peterblockman
Copy link

Still an issue in 2021. Any update on this? no such type: google.protobuf.Empty

@vanthome
Copy link

This is a severe issue. Why is it even closed?

@torkelrogstad
Copy link

Ping for severe issue - any idea how this might be fixed?

@torkelrogstad
Copy link

I was able to fix this by making sure my Proto imports were sorted. I have no idea how or why that fixes it.

@JoshuaWise
Copy link
Contributor

+1

@vsuharnikov
Copy link

vsuharnikov commented May 23, 2022

I had the same issue today with @grpc/proto-loader: "no such type: google.protobuf.StringValue". Probably, it is relevant.

In a proto file I have:

// test.proto
// import ... // other local imports (1)
import "google/protobuf/wrappers.proto";
  "dependencies": {
    "@grpc/proto-loader": "^0.6.11",

I've solved the error by providing a list of all proto files:

const protoLoader = require('@grpc/proto-loader');
const packageDefinition = protoLoader.loadSync(
    [/* all proto files */], // <--- here we should provide test.proto and other files from (1)
    {
        keepCase: true,
        longs: String,
        enums: String,
        defaults: true,
        oneofs: true
    }
);

So in a my case it was an unclear error message. In the reality, proto-loader couldn't find declarations from other imports.

Also, I found wrappers.proto in google-gax. If you still face the issue, you may also try to include proto files from this library (node_modules/google-gax/...).

Hope, this will help someone.

@vokilam-d
Copy link

vokilam-d commented Jun 1, 2022

Thanks, @classLfz, this is working great for me. Only thing I added support for null values.

Also, if someone is interested I built deserialization function (to use in client) based on @classLfz's serialialization.

Whole code:

const isObject = (obj: any): boolean => typeof obj === 'object' && !Array.isArray(obj) && obj !== null;

enum FieldName {
  Number = 'numberValue',
  String = 'stringValue',
  Boolean = 'boolValue',
  Null = 'nullValue',
  List = 'listValue',
  Struct = 'structValue',
}

const typeofFieldNameMap = {
  number: FieldName.Number,
  string: FieldName.String,
  boolean: FieldName.Boolean,
}

const baseFieldNameConstructorMap = {
  [FieldName.Number]: Number,
  [FieldName.String]: String,
  [FieldName.Boolean]: Boolean,
}

const nullFieldValue = 0;

export const serializeGoogleStructValue = (val: any, sub = false) => {
  if (val === null || val === undefined) {
    return {
      [FieldName.Null]: nullFieldValue
    };
  }

  const typeofVal = typeof val;
  if (Object.keys(typeofFieldNameMap).includes(typeofVal)) {
    return {
      [typeofFieldNameMap[typeofVal]]: val
    };
  }

  if (Array.isArray(val)) {
    const out = {
      [FieldName.List]: {
        values: []
      }
    };
    for (const valItem of val) {
      const itemVal = serializeGoogleStructValue(valItem, true);
      out[FieldName.List].values.push(itemVal);
    }
    return out;
  }

  if (typeofVal === 'object') {
    const out = sub ? {
      [FieldName.Struct]: {
        fields: {}
      }
    } : {
      fields: {}
    }

    for (const field of Object.keys(val)) {
      if (val[field] === undefined) {
        continue;
      }

      if (sub) {
        out[FieldName.Struct].fields[field] = serializeGoogleStructValue(val[field], true);
      } else {
        out.fields[field] = serializeGoogleStructValue(val[field], true);
      }
    }
    
    return out;
  }
}

export const deserializeGoogleStructValue = (val: any, sub = false) => {
  if (sub === false && !isObject(val?.fields)) {
    throw new Error(`Invalid Struct format. Object must include "fields" property`);
  }

  if (!isObject(val)) {
    throw new Error(`Invalid Struct format. "${JSON.stringify(val)}" must be an object`);
  }

  const fieldName = Object.keys(val)[0];
  if (fieldName === FieldName.Null) {
    return null;
  }

  const baseValueTypeConstructor = baseFieldNameConstructorMap[fieldName];
  if (baseValueTypeConstructor) {
    return baseValueTypeConstructor(val[fieldName]);
  }

  if (fieldName === FieldName.List) {
    return val[fieldName].values.map(listValue => deserializeGoogleStructValue(listValue, true));
  }

  if (fieldName === FieldName.Struct) {
    return deserializeGoogleStructValue(val[fieldName], true);
  }

  if (isObject(val.fields)) {
    const result = {};
    Object.keys(val.fields).forEach(fieldName => {
      result[fieldName] = deserializeGoogleStructValue(val.fields[fieldName], true);
    });
    return result;
  }
}

copybaranaut pushed a commit to pixie-io/pixie that referenced this issue Jun 22, 2022
Summary:
The underlying tooling seems to be sensitive to order. This works around their bugs.
See also: protobufjs/protobuf.js#1042 (comment)

Test Plan: Ran the script.

Reviewers: philkuz, jamesbartlett, nlanam

Reviewed By: jamesbartlett

Signed-off-by: Vihang Mehta <vihang@pixielabs.ai>

Differential Revision: https://phab.corp.pixielabs.ai/D11656

GitOrigin-RevId: d8d12d0
copybaranaut pushed a commit to pixie-io/pxapi.go that referenced this issue Jun 22, 2022
Summary:
The underlying tooling seems to be sensitive to order. This works around their bugs.
See also: protobufjs/protobuf.js#1042 (comment)

Test Plan: Ran the script.

Reviewers: philkuz, jamesbartlett, nlanam

Reviewed By: jamesbartlett

Signed-off-by: Vihang Mehta <vihang@pixielabs.ai>

Differential Revision: https://phab.corp.pixielabs.ai/D11656

GitOrigin-RevId: 2ae7c8964b9d00e358481a6a5b4c4943603e0caa
orishuss pushed a commit to orishuss/pixie that referenced this issue Jul 14, 2022
Summary:
The underlying tooling seems to be sensitive to order. This works around their bugs.
See also: protobufjs/protobuf.js#1042 (comment)

Test Plan: Ran the script.

Reviewers: philkuz, jamesbartlett, nlanam

Reviewed By: jamesbartlett

Signed-off-by: Vihang Mehta <vihang@pixielabs.ai>

Differential Revision: https://phab.corp.pixielabs.ai/D11656

GitOrigin-RevId: d8d12d0
orishuss pushed a commit to orishuss/pixie that referenced this issue Jul 14, 2022
Summary:
The underlying tooling seems to be sensitive to order. This works around their bugs.
See also: protobufjs/protobuf.js#1042 (comment)

Test Plan: Ran the script.

Reviewers: philkuz, jamesbartlett, nlanam

Reviewed By: jamesbartlett

Signed-off-by: Vihang Mehta <vihang@pixielabs.ai>

Differential Revision: https://phab.corp.pixielabs.ai/D11656

GitOrigin-RevId: d8d12d0
orishuss pushed a commit to orishuss/pixie that referenced this issue Jul 14, 2022
Summary:
The underlying tooling seems to be sensitive to order. This works around their bugs.
See also: protobufjs/protobuf.js#1042 (comment)

Test Plan: Ran the script.

Reviewers: philkuz, jamesbartlett, nlanam

Reviewed By: jamesbartlett

Signed-off-by: Vihang Mehta <vihang@pixielabs.ai>

Differential Revision: https://phab.corp.pixielabs.ai/D11656

GitOrigin-RevId: d8d12d0
@hongsh93
Copy link

any updates?

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

No branches or pull requests