fix(e2e): harden failure lifecycle diagnostics (#38896)

.github/workflows/web-e2e.yml

@@ -97,6 +97,11 @@ jobs:
             mv cucumber-report cucumber-report-non-external
           fi
 
+          if [[ -d .logs ]]; then
+            rm -rf .logs-non-external
+            mv .logs .logs-non-external
+          fi
+
           trap 'vp run e2e:middleware:down' EXIT
           vp run e2e:middleware:up
           vp run e2e:external:prepare
@@ -117,5 +122,8 @@ jobs:
         uses: actions/upload-artifact@043fb46d1a93c77aae656e7c1c64a875d1fc6a0a # v7.0.1
         with:
           name: e2e-logs
-          path: e2e/.logs
+          path: |
+            e2e/.logs/*.log
+            e2e/.logs-non-external/*.log
+          include-hidden-files: true
           retention-days: 7

e2e/AGENTS.md

@@ -127,6 +127,7 @@ This removes:
 - `docker/volumes/plugin_daemon`
 - `e2e/.auth`
 - `e2e/.logs`
+- `e2e/.logs-non-external`
 - `e2e/cucumber-report`
 
 Start the full middleware stack:
@@ -171,6 +172,7 @@ Artifacts and diagnostics:
 - `cucumber-report/artifacts/`: failure screenshots and HTML captures
 - `.logs/cucumber-api.log`: backend startup log
 - `.logs/cucumber-web.log`: frontend startup log
+- `.logs-non-external/`: non-external logs preserved before an external CI run
 
 Open the HTML report locally with:
 

e2e/features/step-definitions/agent-v2/configure.steps.ts

@@ -32,19 +32,6 @@ import {
 
 const concurrentAgentPrompts = [concurrentFirstAgentPrompt, concurrentSecondAgentPrompt]
 
-function attachPageDiagnostics(world: DifyWorld, page: Page) {
-  page.setDefaultTimeout(30_000)
-  page.on('console', (message) => {
-    if (message.type() === 'error') world.consoleErrors.push(message.text())
-  })
-  page.on('pageerror', (error) => {
-    world.pageErrors.push(error.message)
-  })
-  page.on('download', (dl) => {
-    world.capturedDownloads.push(dl)
-  })
-}
-
 const getPromptEditor = (page: Page) =>
   page.getByRole('region', { name: 'Prompt' }).getByRole('textbox', { name: 'Prompt' })
 
@@ -242,7 +229,6 @@ When('I open the same Agent v2 configure page in another tab', async function (t
 
   const agentId = getCurrentAgentId(this)
   const concurrentPage = await this.context.newPage()
-  attachPageDiagnostics(this, concurrentPage)
   this.agentBuilder.configure.concurrentPage = concurrentPage
 
   await concurrentPage.goto(getAgentConfigurePath(agentId))

e2e/features/step-definitions/apps/delete-app.steps.ts

@@ -23,13 +23,15 @@ When('I confirm the deletion', async function (this: DifyWorld) {
 
 Then('the app should no longer appear in the apps console', async function (this: DifyWorld) {
   const appName = this.lastCreatedAppName
-  if (!appName) {
+  const appId = this.createdAppIds.at(-1)
+  if (!appName || !appId) {
     throw new Error(
-      'No app name stored. Run "there is an existing E2E app available for testing" first.',
+      'No app stored. Run "there is an existing E2E app available for testing" first.',
     )
   }
 
   await expect(this.getPage().getByRole('link', { name: appName, exact: true })).not.toBeVisible({
     timeout: 10_000,
   })
+  this.createdAppIds = this.createdAppIds.filter((createdAppId) => createdAppId !== appId)
 })

e2e/features/support/hooks.ts

@@ -1,5 +1,6 @@
 import type { Browser, Page } from '@playwright/test'
 import type { Buffer } from 'node:buffer'
+import type { CleanupTask } from '../../support/cleanup'
 import type { DifyWorld } from './world'
 import { mkdir, writeFile } from 'node:fs/promises'
 import path from 'node:path'
@@ -8,6 +9,7 @@ import { After, AfterAll, Before, BeforeAll, setDefaultTimeout, Status } from '@
 import { chromium } from '@playwright/test'
 import { AUTH_BOOTSTRAP_TIMEOUT_MS, ensureAuthenticatedState } from '../../fixtures/auth'
 import { deleteTestApp } from '../../support/api'
+import { runCleanupTasks } from '../../support/cleanup'
 import { deleteTestDataset } from '../../support/datasets'
 import { getVoiceInputTestMaterialPath } from '../../support/test-materials'
 import { deleteBuiltinToolCredential } from '../../support/tools'
@@ -21,6 +23,9 @@ import {
 
 const e2eRoot = fileURLToPath(new URL('../..', import.meta.url))
 const artifactsDir = path.join(e2eRoot, 'cucumber-report', 'artifacts')
+const closeSessionHookTimeoutMs = 30_000
+const cleanupHookTimeoutMs = 120_000
+const diagnosticHookTimeoutMs = 60_000
 
 let browser: Browser | undefined
 let microphoneBrowserPromise: Promise<Browser> | undefined
@@ -83,14 +88,6 @@ const captureDiagnosticPage = async (
   return [screenshotPath, htmlPath]
 }
 
-const recordCleanup = async (errors: string[], label: string, cleanup: () => Promise<void>) => {
-  try {
-    await cleanup()
-  } catch (error) {
-    errors.push(`${label}: ${error instanceof Error ? error.message : String(error)}`)
-  }
-}
-
 BeforeAll({ timeout: AUTH_BOOTSTRAP_TIMEOUT_MS }, async () => {
   await mkdir(artifactsDir, { recursive: true })
 
@@ -143,11 +140,78 @@ Before(async function (this: DifyWorld, { pickle }) {
   console.warn(`[e2e] start ${pickle.name}${tags ? ` ${tags}` : ''}`)
 })
 
-After(async function (this: DifyWorld, { pickle, result }) {
-  const elapsedMs = this.scenarioStartedAt ? Date.now() - this.scenarioStartedAt : undefined
-  const status = result?.status || Status.UNKNOWN
+// Cucumber runs After hooks in reverse registration order: diagnostics, cleanup, then close.
+After(
+  { name: 'Close browser session', timeout: closeSessionHookTimeoutMs },
+  async function (this: DifyWorld, { result }) {
+    const closeErrors = await runCleanupTasks([
+      { label: 'Close browser session', run: () => this.closeSession() },
+    ])
+    if (closeErrors.length === 0) return
+
+    const message = `Cleanup errors:\n${closeErrors.join('\n')}`
+    this.attach(message, 'text/plain')
+    if (result?.status === Status.PASSED) throw new Error(message)
+  },
+)
+
+After(
+  { name: 'Clean up scenario resources', timeout: cleanupHookTimeoutMs },
+  async function (this: DifyWorld, { result }) {
+    const cleanupTasks: CleanupTask[] = [
+      ...this.createdAgentConfigSkills.toReversed().map((skill) => ({
+        label: `Delete Agent config skill ${skill.name}`,
+        run: () => deleteAgentConfigSkill(skill.agentId, skill.name),
+      })),
+      ...this.createdAgentConfigFiles.toReversed().map((file) => ({
+        label: `Delete Agent config file ${file.name}`,
+        run: () => deleteAgentConfigFile(file.agentId, file.name),
+      })),
+      ...this.createdAgentDriveFiles.toReversed().map((file) => ({
+        label: `Delete Agent drive file ${file.key}`,
+        run: () => deleteAgentDriveFile(file.agentId, file.key),
+      })),
+      ...this.createdAppIds.toReversed().map((id) => ({
+        label: `Delete app ${id}`,
+        run: () => deleteTestApp(id),
+      })),
+      ...this.createdAgentIds.toReversed().map((id) => ({
+        label: `Delete Agent ${id}`,
+        run: () => deleteTestAgent(id),
+      })),
+      ...this.createdDatasetIds.toReversed().map((id) => ({
+        label: `Delete dataset ${id}`,
+        run: () => deleteTestDataset(id),
+      })),
+      ...this.createdBuiltinToolCredentials.toReversed().map((credential) => ({
+        label: `Delete builtin tool credential ${credential.provider}/${credential.credentialId}`,
+        run: () => deleteBuiltinToolCredential(credential.provider, credential.credentialId),
+      })),
+    ]
+
+    const cleanupErrors = await runCleanupTasks(cleanupTasks)
+    cleanupErrors.push(...(await this.runRegisteredCleanups()))
+
+    if (cleanupErrors.length === 0) return
+
+    const message = `Cleanup errors:\n${cleanupErrors.join('\n')}`
+    this.attach(message, 'text/plain')
+    if (result?.status === Status.PASSED) throw new Error(message)
+  },
+)
+
+After(
+  { name: 'Capture scenario diagnostics', timeout: diagnosticHookTimeoutMs },
+  async function (this: DifyWorld, { pickle, result }) {
+    const elapsedMs = this.scenarioStartedAt ? Date.now() - this.scenarioStartedAt : undefined
+    const status = result?.status || Status.UNKNOWN
+
+    console.warn(
+      `[e2e] end ${pickle.name} status=${status}${elapsedMs ? ` durationMs=${elapsedMs}` : ''}`,
+    )
+
+    if (!diagnosticArtifactStatuses.has(status)) return
 
-  if (diagnosticArtifactStatuses.has(status)) {
     const artifactPaths: string[] = []
     const artifactErrors: string[] = []
     const diagnosticPages = uniqueDiagnosticPages([
@@ -181,48 +245,8 @@ After(async function (this: DifyWorld, { pickle, result }) {
 
     if (artifactPaths.length > 0)
       this.attach(`Artifacts:\n${artifactPaths.join('\n')}`, 'text/plain')
-  }
-
-  console.warn(
-    `[e2e] end ${pickle.name} status=${status}${elapsedMs ? ` durationMs=${elapsedMs}` : ''}`,
-  )
-
-  const cleanupErrors: string[] = []
-
-  for (const skill of this.createdAgentConfigSkills.toReversed()) {
-    await recordCleanup(cleanupErrors, `Delete Agent config skill ${skill.name}`, () =>
-      deleteAgentConfigSkill(skill.agentId, skill.name),
-    )
-  }
-  for (const file of this.createdAgentConfigFiles.toReversed()) {
-    await recordCleanup(cleanupErrors, `Delete Agent config file ${file.name}`, () =>
-      deleteAgentConfigFile(file.agentId, file.name),
-    )
-  }
-  for (const file of this.createdAgentDriveFiles.toReversed()) {
-    await recordCleanup(cleanupErrors, `Delete Agent drive file ${file.key}`, () =>
-      deleteAgentDriveFile(file.agentId, file.key),
-    )
-  }
-  for (const id of this.createdAppIds)
-    await recordCleanup(cleanupErrors, `Delete app ${id}`, () => deleteTestApp(id))
-  for (const id of this.createdAgentIds)
-    await recordCleanup(cleanupErrors, `Delete Agent ${id}`, () => deleteTestAgent(id))
-  for (const id of this.createdDatasetIds)
-    await recordCleanup(cleanupErrors, `Delete dataset ${id}`, () => deleteTestDataset(id))
-  for (const credential of this.createdBuiltinToolCredentials.toReversed()) {
-    await recordCleanup(
-      cleanupErrors,
-      `Delete builtin tool credential ${credential.provider}/${credential.credentialId}`,
-      () => deleteBuiltinToolCredential(credential.provider, credential.credentialId),
-    )
-  }
-  if (cleanupErrors.length > 0)
-    this.attach(`Typed cleanup errors:\n${cleanupErrors.join('\n')}`, 'text/plain')
-  await this.runRegisteredCleanups()
-
-  await this.closeSession()
-})
+  },
+)
 
 AfterAll(async () => {
   const microphoneBrowser = await microphoneBrowserPromise?.catch(() => undefined)

e2e/features/support/world.ts

@@ -1,8 +1,9 @@
 import type { IWorldOptions } from '@cucumber/cucumber'
-import type { Browser, BrowserContext, ConsoleMessage, Download, Page } from '@playwright/test'
+import type { Browser, BrowserContext, Download, Page } from '@playwright/test'
 import type { AuthSessionMetadata } from '../../fixtures/auth'
 import { setWorldConstructor, World } from '@cucumber/cucumber'
 import { authStatePath, readAuthSessionMetadata } from '../../fixtures/auth'
+import { runCleanupTasks } from '../../support/cleanup'
 import { baseURL, defaultLocale } from '../../test-env'
 
 export type ScenarioCleanup = () => Promise<void> | void
@@ -128,18 +129,16 @@ export class DifyWorld extends World {
       ...(authenticated ? { storageState: authStatePath } : {}),
     })
     this.context.setDefaultTimeout(30_000)
-    this.page = await this.context.newPage()
-    this.page.setDefaultTimeout(30_000)
-
-    this.page.on('console', (message: ConsoleMessage) => {
+    this.context.on('console', (message) => {
       if (message.type() === 'error') this.consoleErrors.push(message.text())
     })
-    this.page.on('pageerror', (error) => {
-      this.pageErrors.push(error.message)
+    this.context.on('weberror', (webError) => {
+      this.pageErrors.push(webError.error().message)
     })
-    this.page.on('download', (dl) => {
+    this.context.on('download', (dl) => {
       this.capturedDownloads.push(dl)
     })
+    this.page = await this.context.newPage()
   }
 
   async startAuthenticatedSession(browser: Browser) {
@@ -165,27 +164,25 @@ export class DifyWorld extends World {
     this.scenarioCleanups.push(cleanup)
   }
 
-  async runRegisteredCleanups() {
-    const errors: string[] = []
-
-    for (const cleanup of this.scenarioCleanups.toReversed()) {
-      try {
-        await cleanup()
-      } catch (error) {
-        errors.push(error instanceof Error ? error.message : String(error))
-      }
-    }
-
-    if (errors.length > 0) this.attach(`Cleanup errors:\n${errors.join('\n')}`, 'text/plain')
+  runRegisteredCleanups() {
+    return runCleanupTasks(
+      this.scenarioCleanups.toReversed().map((run, index) => ({
+        label: `Registered cleanup ${index + 1}`,
+        run,
+      })),
+    )
   }
 
   async closeSession() {
-    await this.context?.close()
-    this.context = undefined
-    this.page = undefined
-    this.session = undefined
-    this.scenarioStartedAt = undefined
-    this.resetScenarioState()
+    try {
+      await this.context?.close()
+    } finally {
+      this.context = undefined
+      this.page = undefined
+      this.session = undefined
+      this.scenarioStartedAt = undefined
+      this.resetScenarioState()
+    }
   }
 }
 

e2e/scripts/setup.ts

@@ -55,6 +55,7 @@ const e2eStatePaths = [
   path.join(e2eDir, '.auth'),
   path.join(e2eDir, 'cucumber-report'),
   path.join(e2eDir, '.logs'),
+  path.join(e2eDir, '.logs-non-external'),
   path.join(e2eDir, 'playwright-report'),
   path.join(e2eDir, 'test-results'),
 ]

e2e/support/api.ts

@@ -154,7 +154,8 @@ export async function syncAgentV2WorkflowDraft(appId: string, agentId: string):
 export async function deleteTestApp(id: string): Promise<void> {
   const ctx = await createApiContext()
   try {
-    await ctx.delete(`/console/api/apps/${id}`)
+    const response = await ctx.delete(`/console/api/apps/${id}`)
+    await expectApiResponseOK(response, `Delete app ${id}`)
   } finally {
     await ctx.dispose()
   }

e2e/support/cleanup.ts

@@ -0,0 +1,18 @@
+export type CleanupTask = {
+  label: string
+  run: () => Promise<void> | void
+}
+
+export async function runCleanupTasks(tasks: CleanupTask[]): Promise<string[]> {
+  const errors: string[] = []
+
+  for (const task of tasks) {
+    try {
+      await task.run()
+    } catch (error) {
+      errors.push(`${task.label}: ${error instanceof Error ? error.message : String(error)}`)
+    }
+  }
+
+  return errors
+}

e2e/tests/cleanup.test.ts

@@ -0,0 +1,49 @@
+import { describe, expect, it, vi } from 'vitest'
+import { runCleanupTasks } from '../support/cleanup'
+
+describe('runCleanupTasks', () => {
+  it('runs every task in order', async () => {
+    const order: string[] = []
+
+    const errors = await runCleanupTasks([
+      {
+        label: 'first',
+        run: () => {
+          order.push('first')
+        },
+      },
+      {
+        label: 'second',
+        run: async () => {
+          order.push('second')
+        },
+      },
+    ])
+
+    expect(order).toEqual(['first', 'second'])
+    expect(errors).toEqual([])
+  })
+
+  it('continues after failures and labels every error', async () => {
+    const finalTask = vi.fn()
+
+    const errors = await runCleanupTasks([
+      {
+        label: 'sync cleanup',
+        run: () => {
+          throw new Error('sync failure')
+        },
+      },
+      {
+        label: 'async cleanup',
+        run: async () => {
+          throw new Error('async failure')
+        },
+      },
+      { label: 'final cleanup', run: finalTask },
+    ])
+
+    expect(finalTask).toHaveBeenCalledOnce()
+    expect(errors).toEqual(['sync cleanup: sync failure', 'async cleanup: async failure'])
+  })
+})