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

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

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

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

delijati opened this issue Dec 17, 2016 · 4 comments

Comments

@delijati
Copy link

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
Copy link
Member

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
Copy link
Member

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!

@delijati
Copy link
Author

Ohh cool thanks it works.

@dcodeIO
Copy link
Member

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
Projects
None yet
Development

No branches or pull requests

2 participants