unexpected icon indicating copy to clipboard operation
unexpected copied to clipboard

'to satisfy' with circular data causes "Maximum call stack size exceeded"

Open salomvary opened this issue 4 years ago • 9 comments

Given this added to test/unexpected.spec.js:

describe('bug', () => {
  it('fails for FormData with "Maximum call stack size exceeded"', () => {
    expect(new FormData(), 'to satisfy', new FormData());
  });

  it('fails for URLSearchParams with "Maximum call stack size exceeded"', () => {
    expect(new URLSearchParams(), 'to satisfy', new URLSearchParams());
  });

  it.skip('even worse, this seems to never complete in Jest (Jest neither fails nort exist)', () => {
    expect(new Image(), 'to satisfy', new Image());
  });
});

npx jest -- test/unexpected.spec.js on the latest master of this repo fails with an exception:

npx jest -- test/unexpected.spec.js 

 RUNS  test/unexpected.spec.js
/Users/martonsalomvary/unexpected/lib/workQueue.js:28
        throw that.reason();
        ^

RangeError: Maximum call stack size exceeded
    at prepareStackTrace (internal/errors.js:37:29)
    at Function.getOwnPropertyDescriptor (<anonymous>)
    at Object.propertyIsWritable (/Users/martonsalomvary/unexpected/node_modules/unexpected-bluebird/js/main/es5.js:17:37)
    at canAttachTrace (/Users/martonsalomvary/unexpected/node_modules/unexpected-bluebird/js/main/util.js:248:40)
    at Object.ensureErrorObject (/Users/martonsalomvary/unexpected/node_modules/unexpected-bluebird/js/main/util.js:260:17)
    at Promise._rejectCallback (/Users/martonsalomvary/unexpected/node_modules/unexpected-bluebird/js/main/promise.js:465:22)
    at Promise._resolveFromResolver (/Users/martonsalomvary/unexpected/node_modules/unexpected-bluebird/js/main/promise.js:489:17)
    at new Promise (/Users/martonsalomvary/unexpected/node_modules/unexpected-bluebird/js/main/promise.js:69:37)
    at Function.makePromise [as promise] (/Users/martonsalomvary/unexpected/lib/makePromise.js:17:10)
    at /Users/martonsalomvary/unexpected/lib/assertions.js:1955:36
    at Array.forEach (<anonymous>)
    at Object.handler (/Users/martonsalomvary/unexpected/lib/assertions.js:1936:18)
    at Function.Object.<anonymous>.expectPrototype._executeExpect (/Users/martonsalomvary/unexpected/lib/createTopLevelExpect.js:1462:36)
    at /Users/martonsalomvary/unexpected/lib/createTopLevelExpect.js:1345:20
    at Function.Object.<anonymous>.expectPrototype._callInNestedContext (/Users/martonsalomvary/unexpected/lib/createTopLevelExpect.js:1743:30)
    at wrappedExpect (/Users/martonsalomvary/unexpected/lib/createTopLevelExpect.js:1344:26)
    at /Users/martonsalomvary/unexpected/lib/assertions.js:1960:20
    at /Users/martonsalomvary/unexpected/lib/makePromise.js:65:34
    at tryCatcher (/Users/martonsalomvary/unexpected/node_modules/unexpected-bluebird/js/main/util.js:26:23)
    at Promise._resolveFromResolver (/Users/martonsalomvary/unexpected/node_modules/unexpected-bluebird/js/main/promise.js:476:31)
    at new Promise (/Users/martonsalomvary/unexpected/node_modules/unexpected-bluebird/js/main/promise.js:69:37)
    at Function.makePromise [as promise] (/Users/martonsalomvary/unexpected/lib/makePromise.js:17:10)
    at /Users/martonsalomvary/unexpected/lib/assertions.js:1955:36
    at Array.forEach (<anonymous>)

[TRUNCATED]

Using Node.js v12.14.1.

salomvary avatar Feb 05 '20 10:02 salomvary

Did a bit of debugging and turns out the same happens with any circular structure (FormData is implemented in a circular way):

it('fails for circular structures with "Maximum call stack size exceeded"', () => {
      const circular = {};
      circular.loop = circular;
      const circular2 = {};
      circular2.loop = circular2;
      expect(circular, 'to satisfy', circular2);
});

Not sure what unexpected is expected to do with circular structures, the docs don't say anything about this.

salomvary avatar Feb 05 '20 20:02 salomvary

Hmm, yeah, we don't guard against circularity in the right hand-side of to satisfy. I don't think anyone imagined that being anything but a plain, non-circular object.

It works fine with a circular subject:

const expect = require('unexpected');
const circular = {};
circular.a = circular;
expect(circular, 'to satisfy', {a: {a: {a:{}}}}); // :+1:
expect(circular, 'to satisfy', {a: {a: {b:{}}}});
UnexpectedError: 
expected { a: [Circular] } to satisfy { a: { a: { b: ... } } }

{
  a: {
    a: {
      a: { a: [Circular] }
      // missing b: {}
    }
  }
}

We can probably fix this, but I'm wondering what your use case is? :thinking:

papandreou avatar Feb 06 '20 00:02 papandreou

Hi @papandreou, thanks for the quick response!

I am honored to be the first person throwing a circular structure on the right side of an expectation ;)

My use case was verifying a piece of code that was doing something like this:

const form = new FormData()
form.append('some', 'value')
fetch('some/url', {method: 'post', body: form})

using an expectation roughly like:

const expectedForm = new FormData()
expectedForm.append('some', 'value')
expect(fetchOptions, 'to satisfy', {method: 'post', body: expectedForm})

Turns out FormData is internally implemented in a circular way (at least in jsdom), entries having a reference to the FormData instance or something like that. (This was unknown to me.)

As someone not particularly familiar with unexpected and contributing to a codebase with it already in place, I expected this assertion to either work or shout at me for FormData not being supported. Shouting at me would have not surprised me, I understand supporting any type all the supported JS engines have built in is not feasible.

As this might be a usage edge case fixing should also not be urgent. I did poke around in the source but saw no obvious fix (but have never seen unexpected "from inside" before).

Also, now that I understand what the problem is I can come up with other ways of verifying behavior in my use case.

salomvary avatar Feb 06 '20 08:02 salomvary

We don't support this in in to equal either, but we detect it. I think you would be better of by trying to assert using an object structure for the form.

sunesimonsen avatar Feb 06 '20 17:02 sunesimonsen

Upssy, you can't get at the fields. Let me brew something up for you.

sunesimonsen avatar Feb 06 '20 17:02 sunesimonsen

Try to add the following type to unexpected:

expect.addType({
  name: 'FormData',
  base: 'wrapperObject',
  identify(value) {
    return value && value instanceof FormData;
  },
  prefix: (output, value) => output.jsFunctionName('FormData').text('('),
  suffix: (output, value) => output.text(')'),
  unwrap: form => {
    const result = {};

    Array.from(form.entries()).forEach(([key, value]) => {
      if (typeof result[key] === 'undefined') {
        result[key] = value;
      } else if (typeof result[key] === 'string') {
        result[key] = [result[key], value];
      } else {
        result[key].push(value);
      }
    });

    return result;
  }
});

Then something like this:

const a = new FormData();
a.append('foo', 'bar');
a.append('foo', 'quz');
a.append('bar', 'baz');
expect(a, 'to satisfy', {
  foo: ['bar', 'qux'],
  bar: 'baz'
});

will fail with a diff like this:

Screenshot 2020-02-06 at 19 12 37

sunesimonsen avatar Feb 06 '20 18:02 sunesimonsen

You can also simplify it to just return the entries as an array if the order matters to you.

sunesimonsen avatar Feb 06 '20 18:02 sunesimonsen

@sunesimonsen That looks quite neat, thanks for the suggestion. 👍

We don't support this in in to equal either, but we detect it.

If there is an easy fix to avoid "Maximum call stack size exceeded" and fail with a helpful message on circular data, that would be nice.

salomvary avatar Feb 06 '20 18:02 salomvary

@salomvary I agree that it would be a good idea to detect circular data or the right hand side of the to satisfy assertion.

sunesimonsen avatar Feb 06 '20 18:02 sunesimonsen