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

fix: improve error handling #1884

Closed
danisharora099 opened this issue Mar 4, 2024 · 6 comments
Closed

fix: improve error handling #1884

danisharora099 opened this issue Mar 4, 2024 · 6 comments
Assignees
Labels
enhancement New feature or request

Comments

@danisharora099
Copy link
Collaborator

danisharora099 commented Mar 4, 2024

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 (<anonymous>)
    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/[email protected] 
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
Copy link
Collaborator

fbarbu15 commented Mar 6, 2024

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

@chair28980 chair28980 added the enhancement New feature or request label Mar 6, 2024
@weboko
Copy link
Collaborator

weboko commented Mar 7, 2024

I referred to #1612. Let's see if after that this issue is resolved

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

@weboko weboko self-assigned this Mar 7, 2024
@fbarbu15
Copy link
Collaborator

fbarbu15 commented Mar 8, 2024

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
Copy link
Collaborator

weboko commented Apr 24, 2024

Moving to TODO for now.

@weboko weboko changed the title tests: improve error handling bug: improve error handling Apr 24, 2024
@weboko weboko changed the title bug: improve error handling fix: improve error handling Apr 24, 2024
@danisharora099
Copy link
Collaborator Author

danisharora099 commented Apr 25, 2024

Moving to TODO for now.

@weboko What is the acceptance criteria?

@weboko
Copy link
Collaborator

weboko commented Jul 25, 2024

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

@weboko weboko closed this as completed Jul 25, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
Status: Done
Development

No branches or pull requests

4 participants