Prevent store data leaking across requests (#434)

* poc of nested cache data

* better fake await logic

* pagination type checks

* pagination handlers pull from current values

* fixed session retrieval

* subscribe comes first

* generate req_id without load

* only pagination fails

* first pass at drying req_id logic

* share session store utilities between fetch and subscribe

* fix offset pagination fetch

* better cleanup

* cleanup variable name

* more pageinfo ssr fixes

* remove errant log

* cleanup pageInfos

* paginated fragment pageInfo

* remove unused adapter method

* better protection against fragmentStore.update

* add changeset

* fix bug when blocked csf and pending load overlap

* use sessionStore utilities when computing offsets

* bump worker count when running tests

* update mutation store ; increase worker number

* 30 workers doesn't seem reliable on github actions

* cleanup

* cleanup comment

* even fewer workers?

* more use of session store utils

* simplify the api for session stores

* fixed build errors

* pagination handlers now use fetch

* avoid adding pageInfos to store fields

* tweak expect response

* remove unused imports

* make linter "happy"

* report tests with replay

* dont use replay action

* add step to upload

* fix replay test run id

* cache playwright browsers

* pass record replay api key to upload step

* add tests:replay script

* try different upload replay action

* different yarn cmd to install deps from cache

* use setup-node@v3

* checkout before setup

* more jobs

* tweak workflow defn

* add install deps before integration linter

* add yarn cache to test job

* check before instal... again

* integration linter needs packages built

* integration check needs a generated runtime

* have to build sveltekit too?

* use 10 workers for integration tests

* fail on purpose

* use playwright action

* fix project in playwright action config

* always upload videos

* better test id when pushing to replay

* different id scheme for replay tests

* fix invalid yaml

* dont report to replay

* add workers back

* move replay to separate config file

* fix workflow yaml

* fix import path in playwright replay config

* replay is tricky

* marshal and unmarshal are optional

* add changeset

* check for extra responses

* better cleanup in test util

* all hail the linting overlords

* comment out replay video upload

.changeset/calm-scissors-grow.md

@@ -0,0 +1,5 @@
+---
+'houdini': patch
+---
+
+prevent store information from leaking across requests boundaries

.changeset/khaki-weeks-repeat.md

@@ -0,0 +1,5 @@
+---
+'houdini': patch
+---
+
+updated type definition for config file to allow for missing marshal/unmarshal functions

.github/workflows/tests.yml

@@ -29,13 +29,7 @@ jobs:
                   node-version: 16.14.2
                   cache: 'yarn'
 
-            - uses: actions/cache@v2
-              with:
-                  path: '**/node_modules'
-                  key: ${{ runner.os }}-modules-${{ hashFiles('**/yarn.lock') }}
-
             - name: Install Dependencies
-              if: steps.yarn-cache.outputs.cache-hit != 'true'
               run: yarn install
               env:
                   YARN_ENABLE_IMMUTABLE_INSTALLS: false
@@ -48,16 +42,17 @@ jobs:
         runs-on: ubuntu-latest
 
         steps:
-            - name: Setup Node
-              uses: actions/setup-node@v1
-              with:
-                  node-version: 16.14.2
-
             - name: Checkout source
-              uses: actions/checkout@master
+              uses: actions/checkout@v3
               with:
                   ref: ${{ github.ref }}
 
+            - name: Setup Node
+              uses: actions/setup-node@v3
+              with:
+                  cache: 'yarn'
+                  node-version: 16.14.2
+
             - name: Install Dependencies
               run: yarn install
               env:
@@ -74,40 +69,82 @@ jobs:
         runs-on: ubuntu-latest
 
         steps:
+            - name: Checkout source
+              uses: actions/checkout@v3
+              with:
+                  ref: ${{ github.ref }}
+
             - name: Setup Node
-              uses: actions/setup-node@v1
+              uses: actions/setup-node@v3
               with:
+                  cache: 'yarn'
                   node-version: 16.14.2
 
-            - name: Checkout source
-              uses: actions/checkout@master
+            - name: Cache playwright binaries
+              uses: actions/cache@v2
+              id: playwright-cache
               with:
-                  ref: ${{ github.ref }}
+                  path: |
+                      ~/.cache/ms-playwright
+                  key: cache-playwright-linux-1.21.0
 
-            - name: Install Dependencies
-              run: yarn install
+            - name: Install dependencies
+              run: yarn install --immutable
               env:
                   YARN_ENABLE_IMMUTABLE_INSTALLS: false
 
             - name: Build packages
               run: yarn build
 
             - name: Install Playwright
+              if: steps.playwright-cache.outputs.cache-hit != 'true'
               run: npx playwright install --with-deps
 
             - name: Integration Tests
-              run: yarn tests:integration
+              run: yarn workspace integration tests
+              env:
+                  RECORD_REPLAY_TEST_RUN_ID: ${{ env.GITHUB_SHA }}
+
+            # - name: Upload videos
+            #   uses: replayio/[email protected]
+            #   if: ${{ always() }}
+            #   with:
+            #       api-key: ${{ secrets.RECORD_REPLAY_API_KEY }}
+            #       filter: ${{ 'function($v) { $v.metadata.test.result = "failed" and $v.status = "onDisk" }' }}
+
+    integration_linter:
+        name: Integration Linter
+        runs-on: ubuntu-latest
+
+        steps:
+            - name: Checkout source
+              uses: actions/checkout@v3
+              with:
+                  ref: ${{ github.ref }}
+
+            - name: Setup Node
+              uses: actions/setup-node@v3
+              with:
+                  cache: 'yarn'
+                  node-version: 16.14.2
+
+            - name: Install dependencies
+              run: yarn install
+              env:
+                  YARN_ENABLE_IMMUTABLE_INSTALLS: false
+
+            - name: Build packages
+              run: yarn build
+
+            - name: Generate runtime
+              run: yarn workspace integration generate
+
+            - name: Build kit assets
+              run: yarn workspace integration build
 
             # needs to run after build & houdini generate
             - name: Integration lint
               run: yarn workspace integration lint
 
             - name: Integration check
               run: yarn workspace integration check
-
-            - name: Upload test results
-              if: always()
-              uses: actions/upload-artifact@v2
-              with:
-                  name: playwright-report
-                  path: integration/playwright-report

integration/.eslintrc.cjs

@@ -19,6 +19,7 @@ module.exports = {
   },
   rules: {
     '@typescript-eslint/ban-ts-comment': 'off',
-    '@typescript-eslint/ban-types': 'off'
+    '@typescript-eslint/ban-types': 'off',
+    '@typescript-eslint/no-empty-function': 'off'
   }
 };

integration/package.json

@@ -13,7 +13,8 @@
     "package": "svelte-kit package",
     "previewWeb": "cross-env TZ=utc vite preview --port 3007",
     "preview": "npm run generate && concurrently \"yarn previewWeb\" \"yarn api\" -n \"web,api\" -c \"green,magenta\"",
-    "tests": "playwright test",
+    "tests": "playwright test ",
+    "tests:replay": "playwright test -c ./playwright.replay.config.ts  --reporter=line,@replayio/playwright/reporter --workers 10",
     "check": "svelte-check --tsconfig ./tsconfig.json",
     "check:watch": "svelte-check --tsconfig ./tsconfig.json --watch",
     "lint": "prettier --ignore-path .gitignore --check --plugin-search-dir=. . && eslint --ignore-path .gitignore .",
@@ -22,6 +23,7 @@
   "devDependencies": {
     "@kitql/vite-plugin-watch-and-run": "^0.4.0",
     "@playwright/test": "^1.21.0",
+    "@replayio/playwright": "^0.2.21",
     "@sveltejs/adapter-auto": "1.0.0-next.55",
     "@sveltejs/kit": "1.0.0-next.370",
     "@typescript-eslint/eslint-plugin": "^5.10.1",

integration/playwright.config.ts

@@ -1,6 +1,7 @@
 import type { PlaywrightTestConfig } from '@playwright/test';
 
 const config: PlaywrightTestConfig = {
+  retries: 5,
   reporter: process.env.CI ? [['list'], ['html', { open: 'never' }], ['github']] : [['list']],
   use: {
     screenshot: 'only-on-failure'

integration/playwright.replay.config.ts

@@ -0,0 +1,10 @@
+import type { PlaywrightTestConfig } from '@playwright/test';
+import { devices as replayDevices } from '@replayio/playwright';
+import defaultConfig from './playwright.config.ts';
+
+const config: PlaywrightTestConfig = {
+  ...defaultConfig,
+  use: { ...(replayDevices['Replay Chromium'] as any) }
+};
+
+export default config;

integration/src/lib/utils/testsHelper.ts

@@ -1,6 +1,6 @@
 import { sleep, stry } from '@kitql/helper';
 import type { Page, Response } from '@playwright/test';
-import { expect } from '@playwright/test';
+import { expect, test } from '@playwright/test';
 import { routes } from './routes.js';
 
 export async function expectNoGraphQLRequest(
@@ -41,10 +41,10 @@ export async function expectNoGraphQLRequest(
  */
 export async function expectGraphQLResponse(
   page: Page,
-  selector: string | null,
+  selector?: string | null,
   action: 'click' | 'hover' = 'click'
 ) {
-  const listStr = await expectNGraphQLResponse(page, selector, 1, action);
+  const listStr = await expectNGraphQLResponse(page, selector || null, 1, action);
   return listStr[0];
 }
 
@@ -59,29 +59,62 @@ export async function expectNGraphQLResponse(
   n: number,
   action: 'click' | 'hover' = 'click'
 ) {
-  // let nbRequest = 0;
+  // we are going to wait for n responses or 10seconds (whichever  comes first)
+
+  // a promise that we'll resolve when we have all the responses
+  let resolve: () => void = () => {};
+  let resolved = false;
+  const responsePromise = new Promise<void>((res) => {
+    resolve = res;
+  });
+
+  // keep track of how many responses we've seen
   let nbResponse = 0;
+
+  // and a stringified version of the response
   const listStr: string[] = [];
 
-  // function fnReq(request: any) {
-  //   // console.log('>>', request.method(), request.url());
-  //   if (request.url().endsWith(routes.GraphQL)) {
-  //     nbRequest++;
-  //   }
-  // }
+  let lock = false;
+
+  let waitTime: number | null = null;
+  const start = new Date().valueOf();
 
+  // the function to call on each response
   async function fnRes(response: Response) {
-    // console.log('<<', response.status(), response.url());
-    if (response.url().endsWith(routes.GraphQL)) {
-      nbResponse++;
+    // if the response isn't for our API, don't count it
+    if (!response.url().endsWith(routes.GraphQL)) {
+      return;
+    }
+    if (waitTime === null) {
+      waitTime = new Date().valueOf() - start;
+    }
+
+    while (lock) {
+      await sleep(10);
+    }
+
+    lock = true;
+
+    // increment the count
+    nbResponse++;
+
+    // if we're still waiting for a response, add the body to the list
+    if (nbResponse <= n) {
       const json = await response.json();
       const str = stry(json, 0);
       listStr.push(str as string);
     }
+
+    // if we got enough responses, resolve the promise
+    if (nbResponse == n) {
+      resolved = true;
+      resolve();
+    }
+
+    lock = false;
   }
 
   // Listen
-  // page.on('request', fnReq);
   page.on('response', fnRes);
 
   // Trigger the action
@@ -93,18 +126,33 @@ export async function expectNGraphQLResponse(
     }
   }
 
-  // Wait a bit...
-  await sleep(1111);
+  // wait for the first of 10 seconds or n responses
+  await Promise.race([sleep(10000), responsePromise]);
 
   // Remove listeners
-  // page.removeListener('request', fnReq);
   page.removeListener('response', fnRes);
 
-  // Check if numbers are ok
-  // expect(nbRequest, 'nbRequest').toBe(n);
-  expect(nbResponse, 'nbResponse').toBe(n);
+  // if we got this far without resolving the promise, clean it up
+  if (!resolved) {
+    resolve();
+  }
+
+  // if we have a wait time, then wait
+  if (waitTime !== null) {
+    await sleep(waitTime);
+
+    // if we got an extra request, fail
+    if (nbResponse > n) {
+      throw new Error('Encountered too many responses');
+    }
+  }
+
+  // if we didn't get enough responses, we need to fail the test
+  if (!resolved) {
+    // we failed the test
+    throw new Error('Timeout waiting for api requests');
+  }
 
-  // Sort and return!
   return listStr.sort();
 }
 

integration/src/routes/layouts/layout.spec.ts

@@ -3,9 +3,7 @@ import { routes } from '../../lib/utils/routes.js';
 import {
   clientSideNavigation,
   expectNGraphQLResponse,
-  expectNoGraphQLRequest,
-  expectToBe,
-  navSelector
+  expectNoGraphQLRequest
 } from '../../lib/utils/testsHelper.js';
 
 test.describe('Layout & comp', () => {

integration/src/routes/stores/fragment-null.svelte

@@ -5,5 +5,5 @@
 </script>
 
 <div id="result">
-  {$data}
+  {JSON.stringify($data)}
 </div>

integration/src/routes/stores/pending-load-csf.spec.ts

@@ -0,0 +1,22 @@
+import { test } from '@playwright/test';
+import { routes } from '../../lib/utils/routes.js';
+import {
+  clientSideNavigation,
+  expectGraphQLResponse,
+  expectNoGraphQLRequest,
+  expectToBe
+} from '../../lib/utils/testsHelper.js';
+
+test('Simultaneous Pending Load and CSF', async ({ page }) => {
+  // start off on any page (/stores/network)
+  await page.goto(routes.Stores_Network);
+
+  // go to a page with both loads (should also have a clickable thing)
+  await clientSideNavigation(page, routes.Stores_SSR_UserId_2);
+
+  // we should have gotten a response from the navigation
+  await expectGraphQLResponse(page);
+
+  // make sure we get a response if we click on the button
+  await expectGraphQLResponse(page, 'button[id="refresh-1"]');
+});

package.json

@@ -27,7 +27,7 @@
 	"types": "./build/cmd/index.d.ts",
 	"scripts": {
 		"tests": "NODE_ENV=test node --experimental-vm-modules node_modules/.bin/jest",
-		"tests:integration": "yarn workspace integration tests",
+		"tests:integration": "yarn workspace integration tests --workers 5",
 		"tests:watch": "node --experimental-vm-modules node_modules/.bin/jest  --watch",
 		"build": "concurrently \"npm run build:runtime\" \"npm run build:cmd\" \"npm run build:preprocess\" -n \"run,cmd,pre\" -c \"blue.bold,green.bold,yellow.bold\" && npm run build:typeModule",
 		"build:runtime": "concurrently \"npm run build:runtime:esm\" \"npm run build:runtime:cjs\" -n \"esm,cjs\" -c \"green,yellow\"",

src/cmd/generators/runtime/adapter.ts

@@ -37,7 +37,7 @@ export function goTo(location, options) {
 
 export const isBrowser = process.browser
 
-export const clientStarted = true; // Not tested in Sapper.
+export const clientStarted = true; 
 
 export const isPrerender = false
 `