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

Random Eslint failure since upgrading to v5.2.1 #671

Open
andrewbrennanfr opened this issue Jul 24, 2024 · 17 comments
Open

Random Eslint failure since upgrading to v5.2.1 #671

andrewbrennanfr opened this issue Jul 24, 2024 · 17 comments

Comments

@andrewbrennanfr
Copy link

What version of eslint are you using?
8.57.0
What version of prettier are you using?
Not installed as a dep of our project, but npx prettier --version returns 3.2.5
What version of eslint-plugin-prettier are you using?
5.2.1
Please paste any applicable config files that you're using (e.g. .prettierrc or .eslintrc files)

What source code are you linting?
.vue/.ts
What did you expect to happen?
For the linter to succeed
What actually happened?
In our the CI build, we have (seemingly random - as repeated runs causes it to pass) errors during linting.
Local runs of the linter do not fail, only in the CI.

Our error looks something similar to this (I've removed some details to make it more readable) & mentions synckit which was upgraded in the latest version of eslint-plugin-prettier.

Oops! Something went wrong! :(

ESLint: 8.57.0

TypeError: Cannot read properties of undefined (reading 'message')
Occurred while linting /path/to/file.ts:2
Rule: "prettier/prettier"
    at syncFn (/....../node_modules/.pnpm/[email protected]/node_modules/synckit/lib/index.cjs:362:59)
    at Program (/....../node_modules/.pnpm/[email protected][email protected][email protected][email protected][email protected]/node_modules/eslint-plugin-prettier/eslint-plugin-prettier.js:191:32)
    at ruleErrorHandler (/....../node_modules/.pnpm/[email protected]/node_modules/eslint/lib/linter/linter.js:1076:28)
    at /....../node_modules/.pnpm/[email protected]/node_modules/eslint/lib/linter/safe-emitter.js:45:58
    at Array.forEach (<anonymous>)
    at Object.emit (/....../node_modules/.pnpm/[email protected]/node_modules/eslint/lib/linter/safe-emitter.js:45:38)
    at NodeEventGenerator.applySelector (/....../node_modules/.pnpm/[email protected]/node_modules/eslint/lib/linter/node-event-generator.js:297:26)
    at NodeEventGenerator.applySelectors (/....../node_modules/.pnpm/[email protected]/node_modules/eslint/lib/linter/node-event-generator.js:326:22)
    at NodeEventGenerator.enterNode (/....../node_modules/.pnpm/[email protected]/node_modules/eslint/lib/linter/node-event-generator.js:340:14)
    at CodePathAnalyzer.enterNode (/....../node_modules/.pnpm/[email protected]/node_modules/eslint/lib/linter/code-path-analysis/code-path-analyzer.js:803:23)

Downgrading to the previous version we had installed 5.1.3, looks to fix the issue.

@JounQin
Copy link
Member

JounQin commented Jul 25, 2024

Please provide a runnable reproduction.

@andrewbrennanfr
Copy link
Author

@JounQin I don't have one right now - because I can only get it to fail on our CI build.

I downgraded last week & re-upgraded this week - hoping it was just random. But to no avail.

But judging from the 👍 on my report - seems like I might not be the only one. 👀
@dmeyer-pfg do you have a reproducible case to share?

@andrewbrennanfr
Copy link
Author

Just a heads up that this is still an issue for us ☝️
And judging by the thumbs up - we aren't the only ones.

@JounQin
Copy link
Member

JounQin commented Aug 26, 2024

The thumbs up help nothing, we need a reproduction to debug.

@dan-goswag
Copy link

dan-goswag commented Aug 26, 2024

We're having the same issue, running next lint inside github actions. Works sometimes, fails with this error in others. Not yet able to repro in a consistent way, but will circle back if we manage.

Given that @andrewbrennanfr 's stack trace appears to be erroring at https://github.com/fmal/synckit/blob/bd607595c13c133df7544bb5281fd2e29ddc52ba/src/index.ts#L547, it looks like receiveMessageOnPort is returning undefined in some cases?

This is happening on node v20.16.0 which might be relevant given this PR in the release un-ts/synckit#175.

@andrewbrennanfr which version of node are you seeing the error in?

@JounQin
Copy link
Member

JounQin commented Aug 26, 2024

Maybe you can use resolutions(yarn)/overrides(npm/pnpm) synckit: "0.9.0" to confirm whether un-ts/synckit#175 is related?

@andrewbrennanfr
Copy link
Author

The thumbs up help nothing, we need a reproduction to debug.

As I said, it only happens on our CI, so I don't have an example to share.

We're having the same issue, running next lint inside github actions.

We don't use next, but we do use github actions. So somewhat similar.

which version of node are you seeing the error in?

Node 20.x (~20.11.0) - but I run the same node version locally it has never failed locally.

Maybe you can use resolutions(yarn)/overrides(npm/pnpm) synckit: "0.9.0" to confirm whether un-ts/synckit#175 is related?

This doesn't work at 0.9.0 (passed three times & but failed at least once for me) - but pinning at 0.8.8 does work (ran several times without failure for what it's worth).

It's hard to know, because it's somewhat flaky to reproduce.

But I can say that running the linter on github actions, with node20.11 has at least intermittent errors.

@MDSLKTR
Copy link

MDSLKTR commented Aug 27, 2024

Can confirm the same error happening in CI for us (seemlingy random):


ESLint: 9.9.0

TypeError: Cannot read properties of undefined (reading 'message')
Occurred while linting /home/runner/work/cdm/cdm/shared/utils/index.ts:1
Rule: "prettier/prettier"
    at syncFn (/home/runner/work/cdm/cdm/node_modules/synckit/lib/index.cjs:362:59)
    at Program (/home/runner/work/cdm/cdm/node_modules/eslint-plugin-prettier/eslint-plugin-prettier.js:191:32)
    at ruleErrorHandler (/home/runner/work/cdm/cdm/node_modules/eslint/lib/linter/linter.js:1124:48)
    at /home/runner/work/cdm/cdm/node_modules/eslint/lib/linter/safe-emitter.js:45:58
    at Array.forEach (<anonymous>)
    at Object.emit (/home/runner/work/cdm/cdm/node_modules/eslint/lib/linter/safe-emitter.js:45:38)
    at NodeEventGenerator.applySelector (/home/runner/work/cdm/cdm/node_modules/eslint/lib/linter/node-event-generator.js:297:26)
    at NodeEventGenerator.applySelectors (/home/runner/work/cdm/cdm/node_modules/eslint/lib/linter/node-event-generator.js:326:22)
    at NodeEventGenerator.enterNode (/home/runner/work/cdm/cdm/node_modules/eslint/lib/linter/node-event-generator.js:337:14)
    at runRules (/home/runner/work/cdm/cdm/node_modules/eslint/lib/linter/linter.js:1[16](https://github.com/fadi-nextgen-shop/cdm/actions/runs/10578074065/job/29307463664#step:10:17)8:40)

Packages:
"eslint": "9.9.0",
"eslint-config-prettier": "9.1.0",
"eslint-plugin-prettier": "5.2.1",

Node 20.12.0 and yarn 4.1.1 but no pnp only regular node-modules
Also using github actions running ubuntu-latest default runners

@dan-goswag
Copy link

Can confirm pinned 0.9.0 has failed for me. I have not yet managed to make pinned 0.8.8 fail.

@joaocrleite
Copy link

Here the error is intermittent, sometimes it occurs in the pipeline, sometimes it doesn't.

Packages:

"eslint": "^8.56.0",
"eslint-config-prettier": "^9.1.0",
"eslint-plugin-prettier": "^5.2.1",

Node: v20.16.0
Yarn: v1.22.22
Env: Github Actions

Error:


TypeError: Cannot read properties of undefined (reading 'message')
Oops! Something went wrong! :(

    ESLint: 8.56.0
    
    TypeError: Cannot read properties of undefined (reading 'message')
    Occurred while linting /home/runner/work/project/src/services/model/ExploreItem.ts:1
    Rule: "prettier/prettier"
        at syncFn (/home/runner/work/project/node_modules/synckit/lib/index.cjs:362:59)
        at Program (/home/runner/work/project/node_modules/eslint-plugin-prettier/eslint-plugin-prettier.js:191:32)
        at ruleErrorHandler (/home/runner/work/project/node_modules/eslint/lib/linter/linter.js:1076:28)
        at /home/runner/work/project/node_modules/eslint/lib/linter/safe-emitter.js:45:58
        at Array.forEach (<anonymous>)
        at Object.emit (/home/runner/work/project/node_modules/eslint/lib/linter/safe-emitter.js:45:38)
        at NodeEventGenerator.applySelector (/home/runner/work/project/node_modules/eslint/lib/linter/node-event-generator.js:297:26)
        at NodeEventGenerator.applySelectors (/home/runner/work/project/node_modules/eslint/lib/linter/node-event-generator.js:326:22)
        at NodeEventGenerator.enterNode (/home/runner/work/project/node_modules/eslint/lib/linter/node-event-generator.js:340:14)
        at CodePathAnalyzer.enterNode (/home/runner/work/project/node_modules/eslint/lib/linter/code-path-analysis/code-path-analyzer.js:803:23)
    error Command failed with exit code 2.

@felipelube
Copy link

Here's mine. Seems to only affect the CI environment:

"eslint": "8.57.0",
"eslint-config-prettier": "9.1.0",
"eslint-plugin-prettier": "5.2.1",
"prettier": "3.3.3"
Oops! Something went wrong! :(

ESLint: 8.57.0

TypeError: Cannot read properties of undefined (reading 'message')
Occurred while linting /home/runner/work/project/src/workers/scoring/scoring.triggers.ts:1
Rule: "prettier/prettier"
    at syncFn (/home/runner/work/project/node_modules/synckit/lib/index.cjs:362:59)
    at Program (/home/runner/work/project/node_modules/eslint-plugin-prettier/eslint-plugin-prettier.js:191:32)
    at ruleErrorHandler (/home/runner/work/project/node_modules/eslint/lib/linter/linter.js:1076:28)
    at /home/runner/work/project/node_modules/eslint/lib/linter/safe-emitter.js:45:[58](https://github.com/StudentFinance/mydra-api/actions/runs/10671073917/job/29576215739?pr=980#step:4:59)
    at Array.forEach (<anonymous>)
    at Object.emit (/home/runner/work/project/node_modules/eslint/lib/linter/safe-emitter.js:45:38)
    at NodeEventGenerator.applySelector (/home/runner/work/project/node_modules/eslint/lib/linter/node-event-generator.js:297:26)
    at NodeEventGenerator.applySelectors (/home/runner/work/project/node_modules/eslint/lib/linter/node-event-generator.js:326:22)
    at NodeEventGenerator.enterNode (/home/runner/work/project/node_modules/eslint/lib/linter/node-event-generator.js:340:14)
    at CodePathAnalyzer.enterNode (/home/runner/work/project/node_modules/eslint/lib/linter/code-path-analysis/code-path-analyzer.js:803:23)

@dylang
Copy link

dylang commented Sep 5, 2024

Looks like the error could be in Synckit:
https://github.com/un-ts/synckit/blob/main/src/index.ts#L548

Maybe sometimes receives messages that aren't objects with a message property.

What are those messages and where are they coming from?

@greypants
Copy link

Ran into this issue today as well. Using overrides with "synckit": "0.8.8" does seem to work for me as well. Thanks @JounQin and @andrewbrennanfr for the temporary workaround.

 "overrides": {
    "eslint-plugin-prettier": {
      "synckit": "0.8.8"
    }
  },

@andrewbrennanfr
Copy link
Author

Hi 👋 Just an update here (as I opened this issue).

In the end we've removed this plugin from our stack & gone back to using Prettier as a standalone tool.

For a while we've wanted to just manage the code formatting ourselves with Prettier. And after hitting this issue - it pushed us to just go back to using Prettier instead.

So I'll leave the issue open for others who face the problem. But on our side it's no longer a problem. 🙂

Thanks 👍

@JamesBurnside
Copy link

JamesBurnside commented Oct 11, 2024

From the error call stack, this is the line that is failing: https://github.com/un-ts/synckit/blob/42918e8b340b7b6466a5ba280b946b78735877b3/src/index.ts#L547 where receiveMessageOnPort(mainPort) is undefined

receiveMessageOnPort is a node function that may return undefined (when there is no message to receive), so this seems valid. If anyone could explain why synckit might call it at a time where there is no message and hence returns undefined that would be great, but all I can assume is that GitHub actions causes it to happen more frequently where local environments do not.

Therefore @JounQin it would be prudent to have a undefined check around these lines: https://github.com/un-ts/synckit/blob/42918e8b340b7b6466a5ba280b946b78735877b3/src/index.ts#L545-L547

- const { id, ...message } = (receiveMessageOnPort(mainPort) as { message: WorkerToMainMessage<R> }).message
+ const received = receiveMessageOnPort(mainPort) as { undefined | message?: WorkerToMainMessage<R> };
+ if (!received) {
+   // TODO: Handle undefined
+ } else {
+  // happy path
+  const { id, ...message } = received.message;

as well as maybe assess when this function could be called out of turn?


However, I see a PR has been merged 4 days ago pertaining to this code: un-ts/synckit#184

@JounQin could you comment if this could impact/help resolve this issue based on the stack trace? Perhaps @jedlikowski could help here also?


One last thing around pinning 0.8.8:

Can confirm pinned 0.9.0 has failed for me. I have not yet managed to make pinned 0.8.8 fail.

If 0.9.0 fails and 0.8.8 is succeeding, there's not too much that has changed in synckit since then: un-ts/synckit@v0.8.8...v0.9.0#diff-a2a171449d

Looking at that diff I can't see any reason there would be a regression in those changes.

@JounQin
Copy link
Member

JounQin commented Oct 11, 2024

If we can not determine which change impact the new random behavior, I don't think we can fix it confidentially.


Mostly it could be related to un-ts/synckit#154

cc @onigoetz

Any idea?

@jedlikowski
Copy link

Hey @JamesBurnside, in the mentioned PR, I worked on gracefully handling tasks that timeout in the main process but still successfully finish in the worker process. My initial idea was to create a separate MessageChannel for execution of syncFn but I encountered the same problem as you mention here: no message received on port. I never investigated that in more depth as I found an even better alternative.

Just adding my perspective here :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

10 participants