Not handling some binary responses correctly (with reproduction)

[moltar]

Description

Requests fail when fetching binary data.

The binary data seems to be fetching and recording fine. But then it is somehow wrong, because when passing gzip'ed data to the zlib, it complains that the data is not valid.

So the data is either transformed or cut off in some way.

Shareable Source

See https://github.com/moltar/polly-octet-stream

Error Message & Stack Trace

incorrect header check

Config

Using @spotify/polly-jest-presets.

The config it ships with:

https://github.com/spotify/polly-jest-presets/blob/master/src/index.ts

Dependencies

From: https://github.com/spotify/polly-jest-presets/blob/master/package.json

Copy the @pollyjs dependencies from package.json:

  "dependencies": {
    "@pollyjs/adapter-node-http": "^2.6.0",
    "@pollyjs/core": "^2.6.0",
    "@pollyjs/persister-fs": "^2.6.0",
    "lodash.merge": "^4.6.2",
    "setup-polly-jest": "^0.5.2"
  }

  "devDependencies": {
    "@spotify/web-scripts": "^1.0.0",
    "@types/lodash.merge": "^4.6.6",
    "@types/pollyjs__adapter-node-http": "^2.0.0",
    "@types/pollyjs__core": "^2.3.0",
    "@types/pollyjs__persister-fs": "^2.0.0",
    "moment": "^2.24.0"
  }

Relevant Links

https://github.com/moltar/polly-octet-stream

Environment

Node.js v10.15.3
darwin 19.0.0
npm 6.13.1

[offirgolan]

@moltar are you on v2.7.0 of the node-http-adapter? That version includes fixes to handling binary data.

[moltar]

I have installed the latest (2.7.0) version of the @pollyjs/adapter-node-http, and it has no effect.

My understanding is that the latest version fixed a bug about binary requests, not responses.

Thanks.

[moltar]

Interestingly, the value is stored correctly:

"content": {
  "mimeType": "application/octet-stream",
  "size": 118,
  "text": "1f8b08000000000000008be65250a806620505a5c49292a2cca4d292d414e7fcbcb2d4a2e2ccfcbc62439314252b053333431d88a2e4c4dc82c4ccf43c4f90b0a1a181998591a5b18191a5a1b1b9295045ad0e71e6991a6133cfc8ccd8d8c8d8d0c0c8d8c2dcc8c41c641e572c00c734cd0ea2000000"
}

And fetching via curl:

curl https://scaleleap-pup-screenshots.s3.ca-central-1.amazonaws.com/report2.json.gz | xxd -plain | tr -d '\n'

1f8b08000000000000008be65250a806620505a5c49292a2cca4d292d414e7fcbcb2d4a2e2ccfcbc62439314252b053333431d88a2e4c4dc82c4ccf43c4f90b0a1a181998591a5b18191a5a1b1b9295045ad0e71e6991a6133cfc8ccd8d8c8d8d0c0c8d8c2dcc8c41c641e572c00c734cd0ea2000000

Haven't compared byte to byte, but visually, the strings look identical.

Have compared the strings, they are definitely equal.

[moltar]

What I am noticing:

    const arrBuf = await res.arrayBuffer()
    console.log('byteLength', arrBuf.byteLength)

When running without Polly, the byteLength value is 118, but via Polly it's 236. Somehow the value is doubling up.

Edit: added a test for that in my demo repo

[moltar]

Added a Jest snapshot to the test repo. It has the correct response representation when running without Polly, but with Polly contents seem to be not right. Not sure what the contents represent, but they have a are different value.

[moltar]

This seems to be related!

nock/nock#1021

[RubenVerborgh]

Downgrading @pollyjs/adapter-node-http to v2.5.0 fixes it; the bug first appears in v2.6.0.

[NikolasMsomething]

Also experiencing this issue @moltar. Have you happened to find out anything further? Thanks in advance.

[offirgolan]

Looking into it. Should hopefully have a solution in the next few days.

[offirgolan]

@moltar the reproduction isn't working for me. Getting access denied via the url here

[RubenVerborgh]

Reproducible example:

// test.js
const fetch = require('node-fetch');
const { Polly } = require('@pollyjs/core');
const NodeHttpAdapter = require('@pollyjs/adapter-node-http');
const FsPersister = require('@pollyjs/persister-fs');

Polly.register(NodeHttpAdapter);
Polly.register(FsPersister);

describe('test', () => {
  let polly =  new Polly('test', {
    adapters: ['node-http'],
    persister: 'fs',
  });
  afterAll(() => polly.stop());

  it('test', async () => {
    const response = await fetch('http://perdu.com/');
    const text = await response.text();
    console.log(text);
  });
});

echo '{}' > package.json
npm i @pollyjs/[email protected] @pollyjs/[email protected] @pollyjs/[email protected] [email protected] [email protected]
npx jest test.js

Results in:

FetchError: Invalid response body while trying to fetch http://perdu.com/: incorrect header check

The error disappears when uncommenting the Polly.js block.

This is the best MWE I could find; the bug disappears without jest or node-fetch for me.

[offirgolan]

Went down a pretty deep rabbit hole which led me to the issue coming from here:

https://github.com/nock/nock/blob/fad405aa39b6dd98ecdc70432170e7b31abcbd95/lib/common.js#L479

That line returns false where it would actually return true if they used typeof obj.setEncoding === 'function'. Since we always return a stream as the response, nock isn't handling it properly.

This is now dependent on nock/nock#1850

[offirgolan]

Released with v3.0.2 🎉

[NikolasMsomething]

Thank you very much!

[moltar]

@offirgolan Thanks a ton for chasing it down the hole! :)

[futpib]

Sorry to necrobump, but it's a pity https://github.com/moltar/polly-octet-stream is no longer there, I'm struggling to figure how to mock a binary response and it could've helped me.