Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

🐛 Bug: Setting readOnly properties in breakCircularDeps breaks the reporter in parallel mode with allow-uncaught #5188

Closed
3 of 4 tasks
satyen95 opened this issue Aug 6, 2024 · 6 comments · Fixed by #5212
Assignees
Labels
status: accepting prs Mocha can use your help with this one! type: bug a defect, confirmed by a maintainer

Comments

@satyen95
Copy link

satyen95 commented Aug 6, 2024

Bug Report Checklist

  • I have read and agree to Mocha's Code of Conduct and Contributing Guidelines
  • I have searched for related issues and issues with the faq label, but none matched my issue.
  • I have 'smoke tested' the code to be tested by running it outside the real test suite to get a better sense of whether the problem is in the code under test, my usage of Mocha, or Mocha itself.
  • I want to provide a PR to resolve this

Expected

Tests should run perfectly even if a it block (test) is failing
Error in it block (test) should not break parallel-buffered-runner.js even if --allow-uncaught is passed

Actual

All the tests get executed in the expected behaviour, but before the EVENT_RUN_END the process fails while serializing the results with the below error:

/XXX/node_modules/mocha/lib/nodejs/parallel-buffered-runner.js:357
          throw err;
          
TypeError: Cannot set property maxHeaderSize of #<Object> which has only a getter
    at _breakCircularDeps (/XXX/node_modules/mocha/lib/utils.js:679:16)
    at _breakCircularDeps (/XXXnode_modules/mocha/lib/utils.js:679:18)
    at _breakCircularDeps (/XXX/node_modules/mocha/lib/utils.js:679:18)
    at _breakCircularDeps (/XXX/node_modules/mocha/lib/utils.js:679:18)
    at _breakCircularDeps (/XXX/node_modules/mocha/lib/utils.js:679:18)
    at _breakCircularDeps (/XXX/node_modules/mocha/lib/utils.js:679:18)
    at _breakCircularDeps (/XXX/node_modules/mocha/lib/utils.js:679:18)
    at _breakCircularDeps (/XXX/node_modules/mocha/lib/utils.js:679:18)
    at _breakCircularDeps (/XXX/node_modules/mocha/lib/utils.js:679:18)
    at _breakCircularDeps (/XXX/node_modules/mocha/lib/utils.js:679:18)

Failure point:
Screenshot 2024-08-06 at 3 43 31 PM
Reason: Trying to set a read-only property, which only has a getter

Minimal, Reproducible Example

Config:

  • allow-uncaught: true
  • parallel: true

Library dependency (easy to reproduce):
axios: 1.6.8

Create a suite, create a test in it, and make an incorrect/failing API call

Run the tests it will break

Versions

Mocha version: 10.7.0
Node: v20.15.1
OS: MacOS Sonoma 14.2.1 (23C71)

Additional Info

No response

@satyen95 satyen95 added status: in triage a maintainer should (re-)triage (review) this issue type: bug a defect, confirmed by a maintainer labels Aug 6, 2024
@voxpelli
Copy link
Member

voxpelli commented Aug 6, 2024

This is a regression caused by #5032

@JoshuaKGoldberg
Copy link
Member

JoshuaKGoldberg commented Aug 6, 2024

Could someone (@satyen95?) please post a full reproduction we can work with locally?

We might have just fixed this with #5173 (to be released in a patch version soon). But it's hard to be sure we're doing the same steps as you without an actual reproduction.

https://antfu.me/posts/why-reproductions-are-required#why-reproduction

@satyen95
Copy link
Author

satyen95 commented Aug 7, 2024

@JoshuaKGoldberg You can reproduce the issue via this zip, I have added the code for it

mocha-5188.zip

@satyen95
Copy link
Author

satyen95 commented Aug 7, 2024

@JoshuaKGoldberg
Also, I have a question what is the exact behaviour of allow-uncaught will it stop due expect failures too (inside it block)?

Because the behaviour of it is correct in my main project, Run only fails/breaks when the worker processes close due to failure at setup time & not due to any failures in it block. That's the only reason why I have used allow-uncaught.

But the behaviour is different in the given example, run is stopped even due to failure in it block

@JoshuaKGoldberg
Copy link
Member

@JoshuaKGoldberg You can reproduce the issue via this zip, I have added the code for it

mocha-5188.zip

Great, thanks! I can reproduce the bug with that zip in its included Mocha version, 10.7.0.

Aside, a tip: it's a little better practice to put the reproduction up on somewhere I can see it before unzipping potentially unsafe files to a file system 😄. I personally keep a https://github.com/JoshuaKGoldberg/repros repository; other folks make repositories per-issue or use something like GitHub Gists or Stackblitz.

@JoshuaKGoldberg Also, I have a question what is the exact behaviour of allow-uncaught will it stop due expect failures too (inside it block)?

Because the behaviour of it is correct in my main project, Run only fails/breaks when the worker processes close due to failure at setup time & not due to any failures in it block. That's the only reason why I have used allow-uncaught.

But the behaviour is different in the given example, run is stopped even due to failure in it block

Sorry, I don't understand what you're asking. The grammar is broken and I don't follow what you're trying to ask. Can you rephrase please?


Anyway, the .zip has [email protected] and the crash as described. Trimming some blank lines:

$ npm run test

> [email protected] test
> mocha


before

  Test here should pass
    ✔ 1 to be equal to 1

before

  1) Uncaught error outside test suite

  1 passing (411ms)
  1 failing

  1) Uncaught error outside test suite:
     Uncaught TypeError: Cannot set property maxHeaderSize of #<Object> which has only a getter


[mochawesome] Report JSON saved to /Users/josh/repos/mocha-5188/mochawesome-report/mochawesome.json

[mochawesome] Report HTML saved to /Users/josh/repos/mocha-5188/mochawesome-report/mochawesome.html

Bumping to [email protected] has it hang indefinitely:

 $ npm i [email protected] -D; npm run test

changed 1 package, and audited 117 packages in 783ms

...

> [email protected] test
> mocha


before

  Test here should pass
    ✔ 1 to be equal to 1

before

💡 That's the same behavior as #5209! And, in fact, applying the changes in #5212 to node_modules/mocha fixes the indefinite hanging on my computer... only to get back to the original bug:


  1) Uncaught error outside test suite:
     Uncaught TypeError: Cannot set property maxHeaderSize of #<Object> which has only a getter
      at Array.forEach (<anonymous>)

Running with DEBUG=mocha:runner npm run test shows:

  mocha:runner uncaught(): got truthy exception TypeError: Cannot set property maxHeaderSize of #<Object> which has only a getter
  mocha:runner     at _breakCircularDeps (/Users/josh/repos/mocha-5188/node_modules/mocha/lib/utils.js:679:16)
  mocha:runner     at _breakCircularDeps (/Users/josh/repos/mocha-5188/node_modules/mocha/lib/utils.js:679:18)
  mocha:runner     at _breakCircularDeps (/Users/josh/repos/mocha-5188/node_modules/mocha/lib/utils.js:679:18)
  mocha:runner     at _breakCircularDeps (/Users/josh/repos/mocha-5188/node_modules/mocha/lib/utils.js:679:18)
  mocha:runner     at _breakCircularDeps (/Users/josh/repos/mocha-5188/node_modules/mocha/lib/utils.js:679:18)
  mocha:runner     at _breakCircularDeps (/Users/josh/repos/mocha-5188/node_modules/mocha/lib/utils.js:679:18)
  mocha:runner     at exports.breakCircularDeps (/Users/josh/repos/mocha-5188/node_modules/mocha/lib/utils.js:688:10)
  mocha:runner     at SerializableEvent.serialize (/Users/josh/repos/mocha-5188/node_modules/mocha/lib/nodejs/serializer.js:262:5)
  mocha:runner     at /Users/josh/repos/mocha-5188/node_modules/mocha/lib/nodejs/serializer.js:72:13
  mocha:runner     at Array.forEach (<anonymous>)
  mocha:runner     at SerializableWorkerResult.serialize (/Users/josh/repos/mocha-5188/node_modules/mocha/lib/nodejs/serializer.js:71:17)
  mocha:runner     at serialize (/Users/josh/repos/mocha-5188/node_modules/mocha/lib/nodejs/serializer.js:374:15)
  mocha:runner     at /Users/josh/repos/mocha-5188/node_modules/mocha/lib/nodejs/worker.js:127:28
  mocha:runner     at ParallelBuffered.done (/Users/josh/repos/mocha-5188/node_modules/mocha/lib/nodejs/reporters/parallel-buffered.js:151:5)
  mocha:runner     at done (/Users/josh/repos/mocha-5188/node_modules/mocha/lib/mocha.js:1026:16) +417ms

@JoshuaKGoldberg JoshuaKGoldberg self-assigned this Oct 8, 2024
@JoshuaKGoldberg JoshuaKGoldberg added status: accepting prs Mocha can use your help with this one! and removed status: in triage a maintainer should (re-)triage (review) this issue labels Oct 8, 2024
@JoshuaKGoldberg
Copy link
Member

I updated #5212 to also handle this case. 853dc9a

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
status: accepting prs Mocha can use your help with this one! type: bug a defect, confirmed by a maintainer
Projects
None yet
3 participants