search

waku-org / js-waku

JavaScript implementation of Waku v2
https://js.waku.org
Apache License 2.0
166 stars 42 forks source link

fix: improve error handling #1884

Closed danisharora099 closed 1 month ago

danisharora099 commented 6 months ago
This is a **bug report** ## Problem For our tests suite, we sometimes get vague generic errors from Allure that don't convey any directed information about the origin of the error such as: ``` 1) Uncaught error outside test suite Error: No active suite at AllureReporter.startCase (/home/runner/work/js-waku/js-waku/node_modules/allure-mocha/src/AllureReporter.ts:95:13) at AllureReporter.failTestCase (/home/runner/work/js-waku/js-waku/node_modules/allure-mocha/src/AllureReporter.ts:139:12) at MochaAllureReporter.onFailed (/home/runner/work/js-waku/js-waku/node_modules/allure-mocha/src/MochaAllureReporter.ts:82:23) at ParallelBufferedRunner.emit (node:events:529:35) at ParallelBufferedRunner.emit (node:domain:489:12) at ParallelBufferedRunner.Runner.fail (/home/runner/work/js-waku/js-waku/node_modules/mocha/lib/runner.js:453:8) at ParallelBufferedRunner.Runner._uncaught (/home/runner/work/js-waku/js-waku/node_modules/mocha/lib/runner.js:983:12) at /home/runner/work/js-waku/js-waku/node_modules/mocha/lib/nodejs/parallel-buffered-runner.js:340:18 at Array.forEach () at /home/runner/work/js-waku/js-waku/node_modules/mocha/lib/nodejs/parallel-buffered-runner.js:333:12 at processTicksAndRejections (node:internal/process/task_queues:95:5) npm ERR! Lifecycle script `test:node` failed with error: npm ERR! Error: command failed npm ERR! in workspace: @waku/discovery@0.0.1 npm ERR! at location: /home/runner/work/js-waku/js-waku/packages/discovery ``` ## Proposed Solutions Ensure we have access to stack trace ## Notes Ref: https://github.com/waku-org/js-waku/actions/runs/8110738684/job/22168594550?pr=1876 cc @fbarbu15
fbarbu15 commented 6 months ago

I don't believe that it's related to allure, if we don't use allure reporting we get a similar generic error without no stack trace. I also struggled with those types of failures and when I tried to debug and fix them it seemed to be related to tsconfig checks. Somehow the tests seem to be using tsconfig instead of tsconfig.dev like they should and I don't understand why. I raised this some time ago in js-waku channel and @weboko said he has in mind a fix for this

weboko commented 6 months ago

I referred to https://github.com/waku-org/js-waku/pull/1612. Let's see if after that this issue is resolved

Any steps to repro this kind of errors to verify @fbarbu15 , @danisharora099 ?

fbarbu15 commented 6 months ago

Yes, unfortunately even an unused import will trigger such failure. Steps to repro locally:

  1. Have an unused import in a test file (or even mark a test with .only)
  2. Invoke the tests with CI=True to simulate CI runs ex CI=True npx mocha tests/filter/single_node/ping.node.spec.ts We see the same error as reported in the CI. If we remove the unused report(or the only flag) it will work fine image
weboko commented 4 months ago

Moving to TODO for now.

danisharora099 commented 4 months ago

Moving to TODO for now.

@weboko What is the acceptance criteria?

weboko commented 1 month ago

I was looking into this problem and noticed it happens when something throws outside of test() function. I don't think it is actionable - and the only way to check this is to run tests and see the stack trace.

Resolving this tasks as it is not actionable for now but let's keep an eye on failures in our CI and improve handling in case we observe problems. cc @waku-org/js-waku-developers