Upgrade to react-18 issue on CI

[vbonnefond]

Following react-18 upgrade, I got this error on CI : /builds/nmKybPR1/2/navops/unification/navops/ui/node_modules/yoga-layout-prebuilt/yoga-layout/build/Release/nbind.js:53 throw ex; ^ TypeError [ERR_INVALID_ARG_TYPE]: The "data" argument must be of type string or an instance of Buffer, TypedArray, or DataView. Received undefined at writeFile (node:fs:2106:5) at go$writeFile (/builds/nmKybPR1/2/navops/unification/navops/ui/node_modules/graceful-fs/graceful-fs.js:138:14) at Object.writeFile (/builds/nmKybPR1/2/navops/unification/navops/ui/node_modules/graceful-fs/graceful-fs.js:135:12) at /builds/nmKybPR1/2/navops/unification/navops/ui/node_modules/fs-extra/lib/output/index.js:18:27 at /builds/nmKybPR1/2/navops/unification/navops/ui/node_modules/universalify/index.js:22:54 { code: 'ERR_INVALID_ARG_TYPE' }

[techeverri]

Hey @vbonnefond 👋

The error you got there seems to come from a package called yoga-layout-prebuilt. Do you think this is somehow related to loki? 🤔

[vbonnefond]

Hello, I have the error during yarn loki --requireReference --reactUri file:./storybook --chromeFlags='--headless --disable-gpu --hide-scrollbars --no-sandbox' --verboseRenderer when tests are running. If I remove it, it works fine. If I remove react upgrade, it works fine. Here is more context :

loki test v0.28.1
RUNS chrome.app
RUNS chrome.app/START
PASS chrome.app/START
RUNS chrome.app/FETCH_STORIES
PASS chrome.app/FETCH_STORIES
RUNS chrome.app/TESTS
RUNS chrome.app/chrome.laptop.ci/Icons/Cloud Icons 24
RUNS chrome.app/chrome.laptop.ci/Icons/Cloud Icons 48
RUNS chrome.app/chrome.laptop.ci/Icons/Cloud Icons 96
RUNS chrome.app/chrome.laptop.ci/Components/SearchBox/Default
PASS chrome.app/chrome.laptop.ci/Icons/Cloud Icons 24
PASS chrome.app/chrome.laptop.ci/Components/SearchBox/Default
PASS chrome.app/chrome.laptop.ci/Icons/Cloud Icons 48
/builds/nmKybPR1/0/navops/unification/navops/ui/node_modules/yoga-layout-prebuilt/yoga-layout/build/Release/nbind.js:53
        throw ex;
        ^
TypeError [ERR_INVALID_ARG_TYPE]: The "data" argument must be of type string or an instance of Buffer, TypedArray, or DataView. Received undefined
    at writeFile (node:fs:2106:5)
    at go$writeFile (/builds/nmKybPR1/0/navops/unification/navops/ui/node_modules/graceful-fs/graceful-fs.js:138:14)
    at Object.writeFile (/builds/nmKybPR1/0/navops/unification/navops/ui/node_modules/graceful-fs/graceful-fs.js:135:12)
    at /builds/nmKybPR1/0/navops/unification/navops/ui/node_modules/fs-extra/lib/output/index.js:18:27
    at /builds/nmKybPR1/0/navops/unification/navops/ui/node_modules/universalify/index.js:22:54 {
  code: 'ERR_INVALID_ARG_TYPE'
}
error Command failed with exit code 7.

yoga-layout-prebuilt is used by ink that is used by "@loki/runner@^0.30.3"

[techeverri]

I can confirm that yoga-layout-prebuilt is a transitive dependency of Loki

$ yarn why yoga-layout-prebuilt
yarn why v1.22.15
[1/4] Why do we have the module "yoga-layout-prebuilt"...?
[2/4] Initialising dependency graph...
[3/4] Finding dependency...
[4/4] Calculating file sizes...
=> Found "[email protected]"
info Reasons this module exists
   - "_project_#@loki#runner#ink" depends on it
   - Hoisted from "_project_#@loki#runner#ink#yoga-layout-prebuilt"
info Disk size without dependencies: "652KB"
info Disk size with unique dependencies: "680KB"
info Disk size with transitive dependencies: "680KB"
info Number of shared dependencies: 1
Done in 1.31s.

[oblador]

@vbonnefond Do you force react to be version 18? Npm supports multiple versions so it shouldn't be an issue unless you meddle with the version resolution.

[vbonnefond]

You are right, I'm having 2 react versions in my yarn.lock: React 17.0.2 for loki and React 18.2.0 for the rest. I deactivated loki for now, I will wait for loki to to React 18.2.0

[kirilldronkin]

I recently had a chance to dig into the problem, and I have to say that yoga-layout-prebuilt is not the culprit - it just listens for uncaught exceptions and exits the process when some occur. I confirmed this by stripping the interactive renderer import (it uses ink under the hood) and the error was still there, but at that time it just didn't go through yoga-layout-prebuilt.

The root cause of the problem is a flaw in the handling of load timeouts by captureScreenshotForStory from @loki/target-chrome-core. When a timeout exception is thrown it bypasses re-throwing and just prints a debug message, letting undefined to be returned. And then this undefined tries to be written to the file system instead of screenshot buffer by outputFile from fs-extra, which we can see in stack traces.

I don't know why exactly such timeout errors started to occur, they didn't until recently, but fixing the handling of load timeouts along with setting chromeRetries worked for me.

[donaldpipowitch]

We see this error with Node 18 and React 17. It doesn't look like it is fixable by chromeRetries and I couldn't figure out so far what the problem is, sadly.

[florian-milky]

fixing the handling of load timeouts along with setting chromeRetries worked for me

@kirilldronkin how did you do that? Loki suddenly started to be very slow for me and I get the same issue.

[kirilldronkin]

@florian-milky here's the patch, hope it can help you.

diff --git a/node_modules/@loki/target-chrome-core/src/create-chrome-target.js b/node_modules/@loki/target-chrome-core/src/create-chrome-target.js
index 69878ef..de095d9 100644
--- a/node_modules/@loki/target-chrome-core/src/create-chrome-target.js
+++ b/node_modules/@loki/target-chrome-core/src/create-chrome-target.js
@@ -250,11 +250,7 @@ function createChromeTarget(
       }
     };
 
-    client.captureScreenshot = withRetries(
-      options.chromeRetries,
-      CAPTURING_SCREENSHOT_RETRY_BACKOFF
-    )(
-      withTimeout(
+    client.captureScreenshot = withTimeout(
         CAPTURING_SCREENSHOT_TIMEOUT,
         'captureScreenshot'
       )(async (selector = 'body') => {
@@ -328,8 +324,7 @@ function createChromeTarget(
         }
 
         return buffer;
-      })
-    );
+      });
 
     return client;
   }
@@ -402,22 +397,24 @@ function createChromeTarget(
       options.chromeSelector;
     const url = getStoryUrl(storyId);
 
-    const tab = await launchNewTab(tabOptions);
-    let screenshot;
-    try {
-      await withTimeout(options.chromeLoadTimeout)(tab.loadUrl(url, selector));
-      screenshot = await tab.captureScreenshot(selector);
-    } catch (err) {
-      if (err instanceof TimeoutError) {
-        debug(`Timed out waiting for "${url}" to load`);
-      } else {
-        throw err;
-      }
-    } finally {
-      await tab.close();
-    }
-
-    return screenshot;
+    return withRetries(options.chromeRetries, CAPTURING_SCREENSHOT_RETRY_BACKOFF)(async () => {
+      const tab = await launchNewTab(tabOptions);
+      let screenshot;
+      try {
+        await withTimeout(options.chromeLoadTimeout)(tab.loadUrl(url, selector));
+        screenshot = await tab.captureScreenshot(selector);
+      } catch (err) {
+        if (err instanceof TimeoutError) {
+          debug(`Timed out waiting for "${url}" to load`);
+        }
+
+        throw err;
+      } finally {
+        await tab.close();
+      }
+
+      return screenshot;
+    })();
   }
 
   return {

[Urugash]

Is this fix going to be included in a new version of Loki ? My team still can't use Loki in their CI pipeline for the moment

[acapro]

Just wanted to post my fix as well for anyone else facing this issue. Thanks @kirilldronkin for the research and explanation! I tried the patch as posted, but for me it didn't work. I solved this a bit different, by just making sure that undefined would not get returned. Here is what I have:

diff --git a/node_modules/@loki/target-chrome-core/src/create-chrome-target.js b/node_modules/@loki/target-chrome-core/src/create-chrome-target.js
index 69878ef..f72ca1e 100644
--- a/node_modules/@loki/target-chrome-core/src/create-chrome-target.js
+++ b/node_modules/@loki/target-chrome-core/src/create-chrome-target.js
@@ -410,6 +410,7 @@ function createChromeTarget(
     } catch (err) {
       if (err instanceof TimeoutError) {
         debug(`Timed out waiting for "${url}" to load`);
+        throw err; // This line is added to rethrow the error instead of just logging it
       } else {
         throw err;
       }
@@ -417,6 +418,10 @@ function createChromeTarget(
       await tab.close();
     }
 
+    if (!screenshot) {
+      throw new Error('Screenshot is undefined'); // Ensure that screenshot is not undefined before returning it
+    }
+
     return screenshot;
   }
 

[Ajdija]

This was a showstopper for us for a long time. Some of the systems work with above patches and some don't

[vbonnefond]

Thank you for the tips, I needed to add --chromeRetries=3 and --chromeConcurrency=1 to make it works :)

[Grantlyk]

I'm still having this issue with Loki, is there likely to be a PR for one of these gists fixing the issue?

[gblessy]

I also have this issue after the update to loki 0.34.0. Is this issue going to be fixed in the next version of loki?

[AvnerCohen]

@oblador Anything on this item ?