search

percy / percy-puppeteer

Visual testing with Puppeteer and Percy
https://percy.io
MIT License
49 stars 16 forks source link

Improve error message when large DOM is sent #70

Closed djones closed 5 years ago

djones commented 5 years ago

When a DOM is between 10mb and 15mb an unclear error message appears. Add better handling around this case by outputting a clear error message.

Robdel12 commented 5 years ago

I think we might already have logging for this, but LOG_LEVEL=debug needs to be enabled: https://github.com/percy/percy-agent/blob/6375078eb74a263d6a6e2e131225e9206f2f1ae7/src/utils/sdk-utils.ts#L25

Might also have to be done upstream from here. I'm going to try and slice some time out today to confirm

Robdel12 commented 5 years ago 
✗ yarn percy
yarn run v1.15.2
$ percy exec -- node ./index.js
[percy] created build #607: [URL]
[percy] percy has started.
[percy] Error posting snapshot to agent
[percy] stopping percy...
[percy] waiting for 0 snapshots to complete...
[percy] done.
[percy] finalized build #607: [URL]
✨  Done in 5.56s.

The LOG_LEVEL=debug logs were HUGE (since the repro uses base64 to create a 13mb DOM). Here's a clipped version: 

➜ (master) ✗ yarn percy:debug
yarn run v1.15.2
$ LOG_LEVEL=debug percy exec -- node ./index.js
[percy] created build #609: [URL]
[percy] -> assetDiscoveryService.puppeteer.launch durationMs=167
[percy] -> assetDiscoveryService.browser.newPagePool durationMs=166
[percy] percy has started.
[percy] Error posting snapshot to http://localhost:5338/percy/snapshot with body: [object Object]
[percy]  message=Request body larger than maxBodyLength limit, stack=Error: Request body larger than maxBodyLength limit
    at RedirectableRequest.write (/Users/robertdeluca/Projects/Work/scratch/project/node_modules/follow-redirects/index.js:105:24)
    at RedirectableRequest.end (/Users/robertdeluca/Projects/Work/scratch/project/node_modules/follow-redirects/index.js:130:10)
    at dispatchHttpRequest (/Users/robertdeluca/Projects/Work/scratch/project/node_modules/axios/lib/adapters/http.js:234:11)
    at new Promise (<anonymous>)
    at httpAdapter (/Users/robertdeluca/Projects/Work/scratch/project/node_modules/axios/lib/adapters/http.js:18:10)
    at dispatchRequest (/Users/robertdeluca/Projects/Work/scratch/project/node_modules/axios/lib/core/dispatchRequest.js:59:10)
    at process._tickCallback (internal/process/next_tick.js:68:7), adapter=function httpAdapter(config) {
  return new Promise(function dispatchHttpRequest(resolve, reject) {
    var data = config.data;
    var headers = config.headers;
    var timer;

    // Set User-Agent (required by some servers)

Notice the log:

[percy] Error posting snapshot to http://localhost:5338/percy/snapshot with body: [object Object]

So we'll need to stringify that & probably console.log it? Or something so they don't have to put LOG_LEVEL=debug in front of their command.

Robdel12 commented 5 years ago

Can confirm adding maxContentLength to our postSnapshot method makes it work 👍 

 ✗ yarn percy
yarn run v1.15.2
$ percy exec -- node ./index.js
[percy] created build #610: [URL]
[percy] percy has started.
[percy] snapshot taken: 'Does this snapshot?'
[percy] stopping percy...
[percy] waiting for 1 snapshots to complete...
[percy] done.
[percy] finalized build #610: [URL]
✨  Done in 25.79s.
Robdel12 commented 5 years ago

I think we should set the limit high in the SDK and allow the API to return the error here. That way we're not trying to check content size in the SDK when the API already does. It'll return a 419 with the error message already

djones commented 5 years ago

Thanks @Robdel12. Pulling a PR for this together shortly.

djones commented 5 years ago

Closing with the release of https://github.com/percy/percy-agent/pull/118

Upgrade to @percy/puppeteer@1.0.6 or higher to be able to snapshot larger ~16mb DOMs.