timostamm / protobuf-ts

Protobuf and RPC for TypeScript
Apache License 2.0
1.1k stars 129 forks source link

Plugin output is unparseable #134

Closed fenos closed 3 years ago

fenos commented 3 years ago

Hello,

I'm hitting a problem with my plugin when the generated code gets larger than a certain amount. I'm getting the following error:

Plugin output is unparseable: \020\001z\266\302\036\n\020service.twirp.tsz\240\302\036
{content_removed}

you can reproduce this simply by trying and returning a large string in the content of a file

"hello".repeat(100000)

I suspect it is something to do with the max buffer size, but so far I haven't found anything to increase that. Any idea?

fenos commented 3 years ago

Related to https://github.com/hopin-team/twirp-ts/issues/18

timostamm commented 3 years ago

Do you know how to reproduce?

This works for me:

mkdir issue134

cd issue134

cat <<EOF > x.proto
syntax = "proto3";

service authentication {
  rpc A1(Am) returns (Am);
  rpc A2(Am) returns (Am);
  rpc A3(Am) returns (Am);
  rpc A4(Am) returns (Am);
  rpc A5(Am) returns (Am);
  rpc A6(Am) returns (Am);
  rpc A7(Am) returns (Am);
  rpc A8(Am) returns (Am);
  rpc A9(Am) returns (Am);
  rpc A10(Am) returns (Am);
  rpc A11(Am) returns (Am);
  rpc A12(Am) returns (Am);
  rpc A13(Am) returns (Am);
  rpc A14(Am) returns (Am);
  rpc A15(Am) returns (Am);
  rpc A16(Am) returns (Am);
  rpc A17(Am) returns (Am);
  rpc A18(Am) returns (Am);
  rpc A19(Am) returns (Am);
  rpc A20(Am) returns (Am);
}

message Am {}
EOF

npm init -y

npm i @protobuf-ts/plugin@next

npx protoc -I . x.proto --ts_out .
fenos commented 3 years ago

Here is how to reproduce it:

git clone -b unparsable-output https://github.com/hopin-team/twirp-ts.git
cd twirp-ts/example && npm i

make protobuf-ts

protobuf-ts seems to work fine, my plugin produces much bigger output, I'm few hours in debugging this but unfortunately haven't found a clue yet

It could definitely be something on my side, but error message wise is very vague

fenos commented 3 years ago

If you remove a couple of rpc methods from the above proto the output becomes smaller and it does output just fine

fenos commented 3 years ago

I created a simpler repo to reproduce the issue: https://github.com/fenos/protobuf-ts-plugin-issue-134

git clone https://github.com/fenos/protobuf-ts-plugin-issue-134
npm i

make generate

As you can see the only file that should be outputted is test.txt with a long output

https://github.com/fenos/protobuf-ts-plugin-issue-134/blob/main/plugin.ts#L18

throws the same error

fenos commented 3 years ago

@timostamm ok, I think I found the problem and the solution.

Essentially process.stdout.write is buffering the output into chunks giving to protoc only the first part of the binary.

using the following line:

process.stdout._handle.setBlocking(true);

Will allow node to unbuffer the stdout call and send the whole binary to protoc, which successfully generate the file.

From what I've read online, that function is meant to be a private property and should't be used in a production app, but I think this shouldn't be a problem for this specific use case as the plugin would be ran only once

fenos commented 3 years ago

@timostamm I think that the above line should go in the @protobuf-ts/plugin-framework as this issue could arise anytime for anyone compiling big profiles with any of your plugins,

If you are happy with that i'd be glad to raise a PR

EDIT:

Or perhaps the right place to add that line could be the entry point of the plugin ex:

#!/usr/bin/env node

process.stdout._handle.setBlocking(true);

require('./build/plugin')
timostamm commented 3 years ago

That is curious. I can repro the error with 20000 * hello, which is ~100kb.

But we are creating much larger code generator responses, for example unittest_enormous_descriptor.proto.

fenos commented 3 years ago

Yhea...

I found the same weird behaviour too for certain files in the range of ~100kb - ~500kb were OK, but not others...

timostamm commented 3 years ago

Thank you for investigating this 🙏

I'd say the workaround belongs into plugin-base, even if it needs some dirty casts. PR is most welcome!

timostamm commented 3 years ago

Released in v2.0.0-alpha.28 in the "next" channel.