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

Add actionable message in async hook timeout error. #4031

Closed
garg3133 opened this issue Feb 22, 2024 · 12 comments · Fixed by #4053
Closed

Add actionable message in async hook timeout error. #4031

garg3133 opened this issue Feb 22, 2024 · 12 comments · Fixed by #4053
Assignees

Comments

@garg3133
Copy link
Member

Currently, the default timeout for hooks is set to 20000 ms, but sometimes execution of hooks might take longer than that (when loading a slow website in before hook for example), and all the user gets is the following error with no clear indication on how to fix it:

image

Here, the timeout of hooks can be modified by setting the asyncHookTimeout globals config, but the user does not know that such a config exists.

So, we should make the user aware of this potential solution in the error message itself, by adding a help property to the error object, which we use to provide actionable error messages to the users (that is, the actions user can take to resolve the issue).

Steps to reproduce

  1. npm init nightwatch@latest <project-name> (with defaults)
  2. cd <project-name>
  3. Add a new test file named asyncHookTest.js inside test folder, with the sample test provided below.
  4. Inside created nightwatch.conf.js file, add a globals property with the following value:
    globals: {
      asyncHookTimeout: 1000
    }
  5. Run npx nightwatch ./test/asyncHookTest.js
  6. The test will fail with the error as shown above.

Sample test

describe('asyncHookTimeout Demo', function() {
  before(()=> browser.navigateTo('https://ecosia.org'));

  after(browser => browser.end());
});

Potential Fix

This issue can be potentially solved by narrowing down where the error is coming from and adding a help property to the error object (check the codebase to look at the other places this has been done already) with the instructions to potentially fix the issue by setting the asyncHookTimeout globals config. Also make sure that the help only shows up in case of timeout due to the hooks only, and not in all the cases.

@garg3133 garg3133 changed the title Add help text in async hook timeout error. Add actionable message in async hook timeout error. Feb 22, 2024
@shubham-2909
Copy link

hii can i work on this issue please?

@garg3133
Copy link
Member Author

Hi, please go through GSoC Contributor Guidance for Nightwatch. It contains details on how the issues will be assigned.

@shubham-2909
Copy link

yeah i read the guidelines already and did setup the project locally i can now work on this issue right?

@garg3133
Copy link
Member Author

garg3133 commented Feb 22, 2024

Yeah, sure. Go ahead!

You don't need to ask before starting to investigate an issue, and as soon as you feel like you are close to a solution, let us know here how you plan to solve the issue and we'll assign the issue to you.

@shubham-2909
Copy link

ok thanks

@SjxSubham
Copy link

Hi, @garg3133 i have checked on that file . currently, the default timeout is set is 15000,
I have run it in my local system with asynchooktime : 1000 as u have described and it works fine without error,

although here it shows 10000 https://nightwatchjs.org/guide/writing-tests/using-test-globals.html .

Will you assign me to this?
i can fix this. :-)

hope i am able to explain u well...

@garg3133
Copy link
Member Author

Hey @SjxSubham, that 15000 timeout is for Nightwatch project's internal unit tests (present under the test directory). For running example tests, Nightwatch uses nightwatch.conf.js file, so the hook timeout needs to be set there, and the test shouldn't work fine on setting asyncHookTimeout to 1000 but should fail with an error, that's the error we want to improve.

I'd suggest you investigate further on this till the point you roughly know what change you'd need to make and where.

@FireNdIce3
Copy link
Contributor

Hi @garg3133 ,
after exploring a bit on the codebase
I found out that timed-callback.js file is generating this error
So there by Including a help message, this issue can be fixed
moreover the condition that it should be showed only in case of timeout is fulfilled here as well
So can I take this issue ? :)
Thanks

@garg3133
Copy link
Member Author

@FireNdIce3 That seems a good enough investigation and you're going in the right direction. I'll assign the issue to you.

@dikwickley
Copy link
Contributor

It had been a few days since there was any update on the previous PR. While playing around i actually found the fix for it, hence raised a PR #4053 .

@FireNdIce3
Copy link
Contributor

hi @dikwickley
just wanted to know how did you find the base hook function
i tried using the debugger but couldn't find it

@dikwickley
Copy link
Contributor

Hey @FireNdIce3 i used debugger only. Following along the code execution.

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

Successfully merging a pull request may close this issue.

5 participants