search

jsreport / nodejs-client

Nodejs remote client for jsreport
MIT License
4 stars 3 forks source link

Fix for chunked response from jsreport server #7

Closed marekkajda closed 6 years ago

marekkajda commented 6 years ago

Probably it was changed in jsreport server to return PDFs in the chunked response. Current version of nodejs-client returns only the last chunk. I tried to make a fix which doesn't broke previous version

bjrmatos commented 6 years ago

hmm can you please describe what is the issue that you are experienced? seems like you are getting some incomplete pdf responses, right? i've experienced something like that sometime ago. the link will show more details about what i've experienced at that time, but the TL;DR is that i was consuming the stream in another tick/loop of the nodejs eventloop so i was loosing data sometimes and creating race conditions.

with the changes in your PR we are now loading all the pdf file in memory, which is not what we desire, we have avoided to concat buffers in a variable and let nodejs's streams handle files more efficiently by using .pipe, so your changes go against that.

can you share some of the code that you are using to call jsreport client? are you using the .body method in the response object of the jsreport client? maybe there is something wrong in the way that you are using the client and we can help to identify that by looking it. remember that you can also not use .body method (if you are using it) and handle the stream yourself in your code, after all the response object is a stream by itself and you can handle the way you want.

marekkajda commented 6 years ago

Oh sorry - I didn't found that closed issue. Ye I have exactly the same problem and pausing request before returning it as a promise resolve that issue. I'm closing the issue.

PS Is it possible to add/upgrade phantom version to 2.x.x in the docker file (full one)? I will create an issue of course if it is

bjrmatos commented 6 years ago

Is it possible to add/upgrade phantom version to 2.x.x in the docker file (full one)? I will create an issue of course if it is

upgrading is not an option because phantom 2 is not stable enough to be considered a drop-in replacement for phantom 1.

however adding phantom 2 to the full docker image can be possible i don't have any objection for that. doing that will increase the final size of the docker build but after all i think it makes sense for the full image to allow rendering with phantom 2. let's wait for @pofider comments and see if he agree too. thanks for the feature suggestion