Repository navigation
test(dashboard): end in-flight scenario routes quietly before closing a page - #6570
Conversation
… a page
The packaged browser lane failed on an unrelated pull request with an
uncaught TargetClosedError raised from durable-goal-order's route handler:
a sidebar-goal-order write was still being proxied when the scenario
closed its page, and the rejection escaped the handler and took the whole
smoke process down while the next scenario was running.
Every scenario closes through the shared session's close(); it now calls
page.unrouteAll({ behavior: "ignoreErrors" }) before page.close(), the
remedy Playwright names in that error, so a request still in flight at
teardown ends without a verdict instead of crashing the lane.
Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Signed-off-by: song <liusongstep@gmail.com>
loopx-agent
left a comment
There was a problem hiding this comment.
Reviewer: model_agent; model=gpt-6.1-sol; provider=OpenAI; runtime_reported; reasoning_effort=max
完整评审 6570@b1b67da4a1165790b9f1721721f8daee06309b20,实际共同基线 7ed24c6c95f9b16f485e57a54d15893ac6079c2f。
动机
维护打包浏览器验收的开发者,需要跑完当前场景并进入下一个场景。
例如侧栏排序的真实 HTTP 代理尚未返回,场景却已关闭页面;旧共享清理器让异步拒绝逃出,下一场景会被前一场景的 TargetClosedError 中断。当前补丁先解除路由,再关闭页面,已结束的场景不再制造这类进程失败。
独立的延迟真实 HTTP 与延后启动代理两组基线/当前对照均验证了这个变化;运行期间的路由异常和显式产品断言在两版仍失败。
本次只判断共享验收清理边界,不把两个本地场景通过解释为整条 hosted lane 恢复、全场景稳定或产品已安装。
改动思路
判断重点是失败发生在哪个阶段。运行中的代理错误应让场景失败;已经结束的场景,其未完成代理不应在页面销毁后反过来终止下一场景。当前方案复用 openWorkspacePage 已返回的 close(),在原 coverage 停止之后、原 page.close() 之前执行 page.unrouteAll({ behavior: "ignoreErrors" })。没有另建执行器、全局吞异常或自动重试。
不改动会保留已复现的进程退出;逐个场景加 catch 会分散同一清理规则。把顺序放在共享会话归属中,能覆盖排序、创建和其它现有消费者,改动理由与范围一致。这里只需要线性清理顺序,文字足够表达,无须额外状态机或图。
具体改动
完整提案只有 examples/personal-workspace-browser/scenario-context.mjs 一处,增加两行解释和一条清理语句,+3/-0。fetchWorkspaceBackend 的请求转发、openWorkspacePage 的错误收集、coverage 起停和返回结构都没有变化。catch 只围绕解除路由,原 page.close() 仍被 await;没有改产品断言或把其结果转为成功。
我检查了直接消费者:navigation-sorting 先执行 runDurableGoalOrder,其中会等待真实私有偏好 HTTP;创建场景使用同一会话清理器。现有覆盖检索未发现第二套相同 guard。按作者限定查询的最近20条PR元数据(时间跨10月14日至15日,非无限历史)包含分片、标签修复和产品改动,没有同形 teardown 拆分批次。这是现有验收设施的维护修复,有实际消费者和维护收益。
没有独立规范定义此 fixture 的精确关闭顺序;结论依据真实基线反例和当前实现,不把作者描述自证为产品规范。提交的 DCO trailer 与完整 diff 检查通过。
对主干的风险
相同 Node24、仓库原有 browser harness、正常新构建打包资产:8 个独立进程对照(base/head × held-fetch/late-start/active-error/assertion),预期结果全部匹配。两种 teardown race 均 base 失败/current 通过;两种负控均 base/current 保持失败。当前 navigation-sorting(含真实私有偏好存储重启)及 goal-workspace-create(一次性真实后端)通过;基线 navigation-sorting 也通过,说明普通绿跑不能替代 race 反例。native premerge 的3 direct+3 selected全部通过,无skip或hold。
受控验证没有用固定延时猜 race:先等真实 HTTP 收到代理请求,再关闭实际页面;第二组先让 callback 进入,关闭后才允许它开始 fetch。基线分别保留 Request context disposed 和 Target page, context or browser has been closed,当前均完成。coverage 开启,因而也经过原停止分支。
反向验证分别在会话仍活跃时抛 ACTIVE_ROUTE_FAILURE,以及主动触发 EXPLICIT_PRODUCT_ASSERTION。两个版本都保留原错误并非零退出;私有验证器仅为清理自有进程捕获 fatal,仍按失败结束。产品断言没有被补丁包住。正常基线排序本来就能绿跑,这也说明没有用一次普通成功替代强反例。两个当前打包场景通过不代表所有 API 都是真实后端;各自 fixture 边界保留。未查询、等待或触发远端CI。
我的整体评价
APPROVE。 修复了明确的共享验收退出边界,范围完整且可单独回退。long_horizon:改善连续场景的可执行性,晚到的旧回调不再污染后续工作;仍不宣称长周期稳定性或 hosted 成本改善。user_experience:产品操作、权限和确认轮次保持,新增人工轮次为0。
相邻重构检查已做:继续使用现有共享 close() 已是最小合适归属,没有需要在本PR追加的框架或跟踪任务。回滚只需撤回这一清理语句。代码批准与合并授权分别判断,本次没有合并。
English verdict: APPROVE - HEAD b1b67da. The shared teardown fix prevents both independently reproduced late-route escapes while preserving active-route errors and explicit assertion failures. Current packaged callers and native premerge pass; no hosted CI or full-lane stability claim.
Summary
The packaged browser lane (
chat-bundle-browser (2)) failed on an unrelated pull request with an uncaughtTargetClosedErrorraised from the durable goal-order helper's route handler (used by thenavigation-sortingscenario): asidebar-goal-orderwrite was still being proxied when the scenario closed its page, the rejection escaped the handler, and it took the whole smoke process down while the next scenario was already running.Every scenario closes through the shared session's
close()inscenario-context.mjs. It now callspage.unrouteAll({ behavior: "ignoreErrors" })beforepage.close(), the remedy Playwright names in that error, so a request still in flight at teardown ends without a verdict instead of crashing the lane. Tests only, one shared line; no product change.Validation
navigation-sorting(dev build) passes with the guard;goal-workspace-create(real disposable Chat/Todo owner proxied through the same helper) passes with it, so normal proxied reads and writes still complete before teardown.route.fetch: Request context disposedsignal with the immutable old shared helper: close the actual workspace session whileroute.fetchawaits a held real HTTP response. The process fails on the old helper and succeeds on this exact head. A separate late-started route case also fails only on the old helper.b1b67da4a1165790b9f1721721f8daee06309b20, the originalnavigation-sortingandgoal-workspace-createpass in development and the compiled packaged frontend. The latter uses a disposable real Chat/Todo owner. Source-bundle integrity passes; no product code, model, active Goal or manual hosted rerun is involved.🤖 Generated with Claude Code