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

Group error: Error: illegal token '{', ';' expected #568

Closed
delijati opened this Issue Dec 17, 2016 · 4 comments

Comments

Projects
None yet
2 participants
@delijati

delijati commented Dec 17, 2016

protobuf.js version: protobufjs@6.2.1

Parsing error

'use strict';

var protobuf = require('protobufjs');

protobuf.load('test.proto', function(err, root) {
    if (err) throw err;
    console.log(root);
});

Proto file:

message BookDetails {
    optional string publisher = 4;
    optional string publicationDate = 5;
    optional string isbn = 6;
    optional int32 numberOfPages = 7;
    optional string subtitle = 8;
    optional string readerUrl = 10;
    optional string downloadEpubUrl = 11;
    optional string downloadPdfUrl = 12;
    optional string acsEpubTokenUrl = 13;
    optional string acsPdfTokenUrl = 14;
    optional bool epubAvailable = 15;
    optional bool pdfAvailable = 16;
    optional string aboutTheAuthor = 17;
    repeated group Identifier = 18 {
        optional int32 type = 19;
        optional string identifier = 20;
    }
    optional bool fixedLayoutContent = 21;
    optional bool audioVideoContent = 22;
    optional bool isAgencyBook = 23;
}

Error:

% node test.js  
/opt/foo/test.js:6
    if (err) throw err;
             ^

Error: illegal token '{', ';' expected (line 15)
    at illegal (/opt/foo/node_modules/protobufjs/src/tokenize.js:57:16)
    at skip (/opt/foo/node_modules/protobufjs/src/tokenize.js:191:19)
    at parseInlineOptions (/opt/foo/node_modules/protobufjs/src/parse.js:411:9)
    at parseField (/opt/foo/node_modules/protobufjs/src/parse.js:284:21)
    at parseType (/opt/foo/node_modules/protobufjs/src/parse.js:248:25)
    at parseCommon (/opt/foo/node_modules/protobufjs/src/parse.js:213:17)
    at parse (/opt/foo/node_modules/protobufjs/src/parse.js:539:21)
    at process (/opt/foo/node_modules/protobufjs/src/root.js:97:48)
    at /opt/foo/node_modules/protobufjs/src/root.js:167:17
    at fetchReadFileCallback (/opt/foo/node_modules/@protobufjs/fetch/index.js:30:19)
@dcodeIO

This comment has been minimized.

Owner

dcodeIO commented Dec 17, 2016

This is because v6 has no support for (long deprecated) legacy groups. If you can, the official guide recommends to use inner messages instead.

More precisely, the parser currently throws for groups and the decoder skips over groups.

dcodeIO added a commit that referenced this issue Dec 18, 2016

@dcodeIO

This comment has been minimized.

Owner

dcodeIO commented Dec 18, 2016

This commit should add support for legacy groups to the parser (splits up the group into an upper cased type with a special group flag and a lower cased field referencing that type) and to the encoder/decoder including fallbacks.

For example

message MyMessage {
  required group MyGroup = 1 {
    required uint32 something = 2;
  }
}

results in a type named MyGroup with MyGroup#group = true and a corresponding field named myGroup referencing MyGroup as its type within the reflection structure of MyMessage.

It passes a simple test case, but there is a chance that I've got the wire format wrong. So please give it a try and let me know!

dcodeIO added a commit that referenced this issue Dec 18, 2016

@delijati

This comment has been minimized.

delijati commented Dec 18, 2016

Ohh cool thanks it works.

@dcodeIO

This comment has been minimized.

Owner

dcodeIO commented Dec 18, 2016

Feel free to reopen when encountering any issues!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment