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

pbts output is corrupted (missing random end of file) #863

Open
vaiwa opened this issue Jul 11, 2017 · 11 comments
Open

pbts output is corrupted (missing random end of file) #863

vaiwa opened this issue Jul 11, 2017 · 11 comments
Labels

Comments

@vaiwa
Copy link

vaiwa commented Jul 11, 2017

protobuf.js version: 6.6.5 and 6.8.0

running this lines of code on MacOs node v8:

var shell = require('shelljs');
shell.exec("node_modules/.bin/pbjs -t static-module -o ./.compiledAdaptor/posapi-model/.generated/PosApi.js ./adaptor/posapi-model/*.proto");
shell.exec("node_modules/.bin/pbts -o ./adaptor/posapi-model/.generated/PosApi.d.ts ./.compiledAdaptor/posapi-model/.generated/PosApi.js");

the output frompbts (./adaptor/posapi-model/.generated/PosApi.d.ts) is corrupted (cut off randomly and missing the end)

@dcodeIO
Copy link
Member

dcodeIO commented Jul 11, 2017

Does this also happen when you run the CLI the intended way?

@dcodeIO dcodeIO added the unsure label Jul 11, 2017
@vaiwa
Copy link
Author

vaiwa commented Jul 12, 2017

yes it does

@vaiwa
Copy link
Author

vaiwa commented Jul 12, 2017

Looking to code

https://github.com/dcodeIO/protobuf.js/blob/master/cli/pbts.js#L113

    function callJsdoc() {

        // There is no proper API for jsdoc, so this executes the CLI and pipes the output
        var basedir = path.join(__dirname, ".");
        var moduleName = argv.name || "null";
        var cmd = "node \"" + require.resolve("jsdoc/jsdoc.js") + "\" -c \"" + path.join(basedir, "lib", "tsd-jsdoc.json") + "\" -q \"module=" + encodeURIComponent(moduleName) + "&comments=" + Boolean(argv.comments) + "\" " + files.map(function(file) { return "\"" + file + "\""; }).join(" ");
        var child = child_process.exec(cmd, {
            cwd: process.cwd(),
            argv0: "node",
            stdio: "pipe",
            maxBuffer: 1 << 24 // 16mb
        });
        var out = [];
        child.stdout.on("data", function(data) {
            out.push(data);
        });
        child.stderr.pipe(process.stderr);
        child.on("close", function(code) {
            ...

the child.on("close"..., is called before in child.stdout.on("data",... was seen all the data

but when executed the cmd in my normal macOs terminal the output in OK

@vaiwa
Copy link
Author

vaiwa commented Jul 12, 2017

Found the issue!
Caused by new version of npmjs.com/package/jsdoc used in child_process call
version [email protected] is fine, but 3.5.1 cause the issue!

I added the version to our package.json, but the automatic install of it was quite a trap! Could you consider not installing it automatically for random version, but add it to package.json so yarn.lock would catch it for next time, and not break it from day to day as happened? Thanks! =)

@buu700
Copy link

buu700 commented Jul 12, 2017

Came here to report the same thing. For some reason, when protobufjs and google-gax are installed at the same time, my pbts output gets randomly truncated around ~1350 lines.

(Not an immediate blocker for me anymore, as switching from the google-cloud meta-package to only what I needed — which happened to not depend on google-gax — seems to have mitigated the issue.)

Relevant output:

gibson@8d3cf534d161:~$ pbts /cyph/shared/js/proto/index.js -o balls.d.ts ; tail balls.d.ts
     * @throws {$protobuf.util.ProtocolError} If required fields are missing
     */
    public static decodeDelimited(reader: ($protobuf.Reader|Uint8Array)): BooleanArray;

    /**
     * Verifies a BooleanArray message.
     * @param message Plain object to verify
     * @returns `null` if valid, otherwise the reason why it is not
     */
    public static gibson@8d3cf534d161:~$

dcodeIO added a commit that referenced this issue Jul 12, 2017
@dcodeIO
Copy link
Member

dcodeIO commented Jul 12, 2017

Does this fix it? Iirc, a similar issue was the reason for the 'maxBuffer' option to be added, so this might still be related somehow.

@buu700
Copy link

buu700 commented Jul 12, 2017

Yep, looks like that fixed it!

Well, mostly. I reproduced the issue the first time I re-ran the command after replacing node_modules/protobufjs/cli/pbts.js, but since then it seems consistently fixed. Is there some kind of caching going on that could explain that? If not, it's possible it was just a local issue with my filesystem or something.

Edit: Actually, upon further investigation. it doesn't seem to be consistently working at all. It works a bit more often than it did before, but still produces the bad output more often than not. Could this be related to yesterday's new release of Node.js?

Edit 2: Nope, pinning to Node 8.1.3 (with and without 952c7d1) didn't have any effect.

@buu700
Copy link

buu700 commented Jul 13, 2017

It takes a bit longer to run, but here's my temporary workaround for anyone else who needs it, which seems to be reliable enough as a short-term fix:

while ! tsc ${output}.d.ts &> /dev/null ; do
    pbts ${output}.js -o ${output}.d.ts
done

@wraith7
Copy link

wraith7 commented Jul 13, 2017

Exactly same issue here.

{ npm: '3.10.10',
ares: '1.10.1-DEV',
http_parser: '2.7.0',
icu: '58.2',
modules: '48',
node: '6.10.2',
openssl: '1.0.2k',
uv: '1.9.1',
v8: '5.1.281.98',
zlib: '1.2.11' }

@vaiwa
Copy link
Author

vaiwa commented Jul 14, 2017

Did you tried to add [email protected] to your package.json?

something is wrong running the newer version it in the child_process.exec, the close signal from the child process came for me before the data are processed, but running it not in child_process works fine

Szpadel pushed a commit to Szpadel/protobuf.js that referenced this issue Jul 25, 2017
@bramblex
Copy link

I got the same problem, but using [email protected] can be fixed

ccdunder pushed a commit to ccdunder/protobuf.js that referenced this issue Nov 27, 2017
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

5 participants