Error: Unexpected state in afterSpecHandler: step-finished

daviddealvarado-clarivate commented 4 months ago

Current behavior

If a cy.task is called like this:

cy.task('getDownloadedFileName', null).then((fileName) => { cy.wrap(fileName).should("not.contain", "TIMEOUT").then((fileName) => { cy.wrap(fileName).as(alias); }); });

Then an exception is raised: An error was thrown in your plugins file while executing the handler for the after:spec event.

The error we received was:

Error: Unexpected state in afterSpecHandler: step-finished (this might be a bug, please report at https://github.com/badeball/cypress-cucumber-preprocessor) at createError (C:\Users\U6073228\projects\test-cypress\node_modules\@badeball\cypress-cucumber-preprocessor\dist\helpers\error.js:9:12) at afterSpecHandler (C:\Users\U6073228\projects\test-cypress\node_modules\@badeball\cypress-cucumber-preprocessor\dist\plugin-event-handlers.js:300:43) at processTicksAndRejections (node:internal/process/task_queues:95:5)

And the execution is aborted.

Desired behavior

The task should finish, an screenshot must be attached and continue the tests.

Test code to reproduce

https://github.com/metuskale/cypress_task_failure

Versions

Cypress version: 13.9.0
Preprocessor version: 20.0.5
Node version: 20.13.1

Checklist

[x] I've read the FAQ.
[x] I've read instructions for logging issues.
[x] I'm not using cypress-cucumber-preprocessor@4.3.1 (package name has changed and it is no longer the most recent version, see #689).

badeball commented 4 months ago

I'm not seeing the same error using your example. It runs seemingly fine, aside from the first test which fails (expectedly) with ENOENT: no such file or directory, scandir 'cypress/downloads'.

FYI, your example doesn't even contain @cypress/browserify-preprocessor as a dependency. Have you actually tried to run the example you're providing me?

daviddealvarado-clarivate commented 4 months ago

Hi! I am not sure about the @cypress/browserify-preprocessor dependency but just to make sure I just copied an example from your repo. The failure of the first test should not be related to the fact of a missing folder (I don't know why it got deleted after my push to the repo because I added a .gitkeep to make sure it wasn't deleted.

Of course I run the example, actually the error codes were copied from that run. I have just push an update adding the dependency and I attach an screenshot of the run failure (the downloads folder is existing):

badeball commented 4 months ago

This happens because you're hogging the plugin thread with sync calls to fs for 100s. Do the calls asynchronously, add some time between each check and reduce 100s to something more appropriate. Then you'll see the errors go away.

badeball commented 4 months ago

--- a/cypress.config.ts
+++ b/cypress.config.ts
@@ -59,7 +59,7 @@ export default defineConfig({
       });

       cypressOn('task', {
-        getDownloadedFileName() {
+        async getDownloadedFileName() {
           let files;
           let start = Date.now();
           let elapsed = Date.now();
@@ -70,7 +70,8 @@ export default defineConfig({
               return str.endsWith('.xlsx') || str.endsWith('.ris') || str.endsWith('.csv');
             });
             elapsed = Date.now() - start;
-          } while (files.length < 1 && elapsed < 100000);
+            await sleep(100);
+          } while (files.length < 1 && elapsed < 1000);

           if (files.length > 0) {
             // It's an array but will/should have only one element
@@ -86,3 +87,7 @@ export default defineConfig({
     }
   },
 });
+
+function sleep (ms) {
+  return new Promise(resolve => setTimeout(resolve, ms))
+}

daviddealvarado-clarivate commented 4 months ago

I agree with the point of using the async call to be able to sleep, I will make that change to be more efficient.

I also understand that it might seem a lot of wait time, but the thing is that in my particular case the file is generated on the fly and may take up to 5m to be ready to be read. Also, I don't the thinks the time waiting is a reason to make the state to be in something unexpected. One could always configure the cy.task timeout to be 100s instead of the standard 30s or 1s if you want and IMHO that shouldn't raise an exception other than a timeout if there is any internal limitation, but not an unexpected behaviour, isn't it?

badeball commented 4 months ago

Also, I don't the thinks the time waiting is a reason to make the state to be in something unexpected.

You can wait for whatever amount you want, but don't hog the plugin thread with sync calls. It makes Cypress behave incorrectly, as shown by timing out on screenshot, as well as on the internal task named cypress-cucumber-preprocessor:test-step-finished.

Said task doesn't actually take up any time. You can set a timeout for as long as you want, but as long as you hog the plugin thread it won't matter. By hogging the thread, you prevent the plugin from doing its job and that's why there's a state error.

daviddealvarado-clarivate commented 4 months ago

Understood. I will try to apply you recommendations and see how it behaves

badeball / cypress-cucumber-preprocessor