alvarcarto / url-to-pdf-api

Web page PDF/PNG rendering done right. Self-hosted service for rendering receipts, invoices, or any content.
MIT License
7.03k stars 779 forks source link

Upgrade puppeteer and ws libraries #60

Closed eriktrom closed 6 years ago

eriktrom commented 6 years ago

fixes #57

Reason: Chrome 63+ does closes socket without waiting for close frame from server. Upstream WS library then throws on server. Puppeteer was upgraded to handle this in the following commits

Trace of this problem can be found by following these links:

Note: there is an upstream change to puppeteer that required changing this https://github.com/alvarcarto/url-to-pdf-api/commit/4c7f47d8feccb2eed8a0e0824e255074215a11a1 - this now logs the source code of the file. Perhaps someone can help out with this, this is my first go at the puppeteer library. The change was required b/c of this assertion in puppeteer 1.x https://github.com/GoogleChrome/puppeteer/blob/c857aef/lib/NavigatorWatcher.js#L28-L30

^^ otherwise, works like a charm, tested with 200 iterations of download pdf from cnbc.com, 0 errors, but don't merge until verbose logging is fixed. (please help someone, i'm out of time for this).

eriktrom commented 6 years ago

I recant - I'm not convinced this fully fixes it - upon running 200 iterations on ubuntu, there are 503's - i've got a debugger running locally, it'll pop after a couple hundred iterations(as it did earlier today) when it does i'll paste a snapshot of the stack frame that is responsible(was the ws node module previously) but that's been updated so... fyi

eriktrom commented 6 years ago

okay so it does now catch the exception - which prevents express from exiting - this stack trace previously was an uncaught exception when using node --inspect

2018-03-03T00:24:32.023Z - error: [error-logger.js] Error: Navigation Timeout Exceeded: 30000ms exceeded
    at Promise.then (/Users/trom/Documents/desktop_space/Dropbox/dev/eriktrom/RepairShopr/url-to-pdf-api/node_modules/puppeteer/lib/NavigatorWatcher.js:71:21)
    at <anonymous> 'Error: Navigation Timeout Exceeded: 30000ms exceeded\n    at Promise.then (/Users/trom/Documents/desktop_space/Dropbox/dev/eriktrom/RepairShopr/url-to-pdf-api/node_modules/puppeteer/lib/NavigatorWatcher.js:71:21)\n    at <anonymous>'

thus, i believe, besides the verbose logging introduced upstream from upgrading puppeteer, that #57 is fixed...

eriktrom commented 6 years ago

nope:

2018-03-03T00:25:48.484Z - error: [error-logger.js] Error: Navigation Timeout Exceeded: 30000ms exceeded
    at Promise.then (/Users/trom/Documents/desktop_space/Dropbox/dev/eriktrom/RepairShopr/url-to-pdf-api/node_modules/puppeteer/lib/NavigatorWatcher.js:71:21)
    at <anonymous> 'Error: Navigation Timeout Exceeded: 30000ms exceeded\n    at Promise.then (/Users/trom/Documents/desktop_space/Dropbox/dev/eriktrom/RepairShopr/url-to-pdf-api/node_modules/puppeteer/lib/NavigatorWatcher.js:71:21)\n    at <anonymous>'
events.js:183
      throw er; // Unhandled 'error' event
      ^

Error: read ECONNRESET
    at _errnoException (util.js:1022:11)
    at TCP.onread (net.js:615:25)
Waiting for the debugger to disconnect...

I'll step through the stack frame on monday, but looks upgrading does not fix this, i'll leave this open for the weekend, maybe more eyeballs will help

eriktrom commented 6 years ago

closing b/c im bumping puppeteers version of ws, and thus trying a fork of it - (I had yarn unlinked puppeteer w/o this locally, thus, the failure above, was a mistake.) if it works running iterations on my server over the weekend, i'll report that back here