From 36ea5dce6335cf0f25ed0528b8ba0bcdfe8fcf71 Mon Sep 17 00:00:00 2001 From: Bob Qiu Date: Tue, 29 Sep 2026 16:30:30 +0800 Subject: [PATCH] fix(project-modal): close on redirect by comparing location, not navigation.state MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit CI timing diagnostics (added in 66a777f86) showed project creation completing in ~13ms with zero lock contention and zero network delay, disproving the two prior hypotheses (lock contention, slow team-projects fetch). The real bug: ProjectModal's close effect watched `useNavigation().state` flipping away from 'idle' to detect the post-create redirect, but a redirect that resolves fast enough can settle back to 'idle' within the same render/commit, so the effect never observes the transient state and the modal never closes — with no error, since the create itself succeeded. Fixed by comparing the pathname captured when the modal opened against the current location instead, which can't be missed regardless of timing. Removes the now-unnecessary temporary timing diagnostics. Keeps the lock-acquire timeout and bounded team-projects fetch timeout as reasonable defensive improvements, even though neither was the actual cause of the CI hangs. Co-Authored-By: Claude Sonnet 5 --- .../node-src/services/project.ts | 10 ------- packages/insomnia/src/common/project.ts | 14 +--------- ...ganization.$organizationId.project.new.tsx | 6 ----- .../ui/components/modals/project-modal.tsx | 26 +++++++++++++++---- 4 files changed, 22 insertions(+), 34 deletions(-) diff --git a/packages/insomnia-data/node-src/services/project.ts b/packages/insomnia-data/node-src/services/project.ts index 8d4b437a44..5567ca0054 100644 --- a/packages/insomnia-data/node-src/services/project.ts +++ b/packages/insomnia-data/node-src/services/project.ts @@ -75,8 +75,6 @@ export async function getFirstProjectOfOrganization(organizationId: string) { const TEAM_PROJECTS_FETCH_TIMEOUT_MS = 8000; export async function getAllTeamProjects(organizationId: string) { - // TEMP DIAGNOSTIC: see matching note in insomnia/src/common/project.ts's wrapWithLock. - const diagStart = Date.now(); const { id: sessionId } = await userSessionService.get(); if (!sessionId) { return []; @@ -84,7 +82,6 @@ export async function getAllTeamProjects(organizationId: string) { console.log('[project] Fetching', organizationId); const response = await fetchTeamProjects({ sessionId, organizationId, timeout: TEAM_PROJECTS_FETCH_TIMEOUT_MS }); - console.log(`[create-project-timing] fetchTeamProjects done after ${Date.now() - diagStart}ms`); return response.data; } @@ -160,21 +157,14 @@ export async function syncTeamProjects({ } export async function syncProjects(organizationId: string) { - // TEMP DIAGNOSTIC: see matching note in insomnia/src/common/project.ts's wrapWithLock. - const diagStart = Date.now(); - console.log(`[create-project-timing] syncProjects(${organizationId}) start`); // Local-only organizations have no team projects to fetch, so bail out before the request. if (models.organization.isLocalOrganizationId(organizationId)) { - console.log('[create-project-timing] syncProjects bailed out (local-only organization)'); return; } const user = await userSessionService.get(); const teamProjects = await getAllTeamProjects(organizationId); // ensure we don't sync projects in the wrong place if (Array.isArray(teamProjects) && user.id) { - const syncStart = Date.now(); await syncTeamProjects({ teamProjects, organizationId }); - console.log(`[create-project-timing] syncTeamProjects() done after ${Date.now() - syncStart}ms`); } - console.log(`[create-project-timing] syncProjects total ${Date.now() - diagStart}ms`); } diff --git a/packages/insomnia/src/common/project.ts b/packages/insomnia/src/common/project.ts index 0a412c566b..18c48c953d 100644 --- a/packages/insomnia/src/common/project.ts +++ b/packages/insomnia/src/common/project.ts @@ -87,23 +87,11 @@ const lockGenerator = () => { const wrapWithLock = Promise>(fn: T): T => { const wrappedFn = async (...args: Parameters): Promise> => { - // TEMP DIAGNOSTIC: timing breakdown to root-cause the intermittent - // "Create project" hang seen in CI (see insomnia PR #10556). Remove - // once the real bottleneck is identified from CI console-log output. - const callId = Math.random().toString(36).slice(2, 8); - const waitStart = Date.now(); - console.log(`[lock-timing] ${callId} waiting for lock`); await lock(); - const waitMs = Date.now() - waitStart; - console.log(`[lock-timing] ${callId} acquired lock after ${waitMs}ms`); try { - const runStart = Date.now(); - const result = await fn(...args); - console.log(`[lock-timing] ${callId} fn() finished after ${Date.now() - runStart}ms (holding lock)`); - return result; + return await fn(...args); } finally { await unlock(); - console.log(`[lock-timing] ${callId} released lock`); } }; return wrappedFn as T; diff --git a/packages/insomnia/src/routes/organization.$organizationId.project.new.tsx b/packages/insomnia/src/routes/organization.$organizationId.project.new.tsx index 16bf47b1c2..5610971e02 100644 --- a/packages/insomnia/src/routes/organization.$organizationId.project.new.tsx +++ b/packages/insomnia/src/routes/organization.$organizationId.project.new.tsx @@ -60,11 +60,7 @@ export const reportGitProjectCount = async (organizationId: string, sessionId: s }; const createProjectImpl = async (organizationId: string, newProjectData: CreateProjectData) => { - // TEMP DIAGNOSTIC: see matching note in ~/common/project.ts's wrapWithLock. - const diagStart = Date.now(); - console.log(`[create-project-timing] createProjectImpl start (storageType=${newProjectData.storageType})`); const user = await services.userSession.get(); - console.log(`[create-project-timing] userSession.get() done after ${Date.now() - diagStart}ms`); const sessionId = user.id; invariant(sessionId, 'User must be logged in to create a project'); invariant( @@ -73,12 +69,10 @@ const createProjectImpl = async (organizationId: string, newProjectData: CreateP ); if (newProjectData.storageType === 'local') { - const dbStart = Date.now(); const project = await services.project.create({ name: newProjectData.name, parentId: organizationId, }); - console.log(`[create-project-timing] services.project.create() done after ${Date.now() - dbStart}ms (total ${Date.now() - diagStart}ms)`); return project._id; } diff --git a/packages/insomnia/src/ui/components/modals/project-modal.tsx b/packages/insomnia/src/ui/components/modals/project-modal.tsx index d4391e6a41..2dfb8468e7 100644 --- a/packages/insomnia/src/ui/components/modals/project-modal.tsx +++ b/packages/insomnia/src/ui/components/modals/project-modal.tsx @@ -2,7 +2,7 @@ import type { StorageRules } from 'insomnia-api'; import type { GitRepository, Project } from 'insomnia-data'; import React, { useEffect, useRef, useState } from 'react'; import { Button, Dialog, Heading, Modal, ModalOverlay } from 'react-aria-components'; -import { useNavigation } from 'react-router'; +import { useLocation } from 'react-router'; import { useActiveView } from '~/ui/components/project/utils'; import { UnsavedChangesGuard } from '~/ui/components/unsaved-changes-guard'; @@ -25,16 +25,32 @@ export const ProjectModal = ({ project?: Project; gitRepository?: GitRepository; }) => { - // Close the modal when a navigation happens - const activeNavigation = useNavigation(); const [hasUnsavedChanges, setHasUnsavedChanges] = useState(false); const modalRef = useRef(null); + // Close the modal once a submission (e.g. creating a project) redirects + // to a new URL. Comparing the pathname captured when the modal opened + // against the current one — rather than watching `useNavigation().state` + // flip away from 'idle' in an effect — because a fast redirect can + // resolve within the same render/commit that starts it, so the 'loading' + // state is never observed and the effect never fires, leaving the modal + // stuck open indefinitely with no error (see insomnia PR #10556). + const location = useLocation(); + const openedAtPathRef = useRef(location.pathname); useEffect(() => { - if (activeNavigation && activeNavigation.state !== 'idle' && activeNavigation.location && isOpen) { + if (isOpen) { + openedAtPathRef.current = location.pathname; + } + // Deliberately only re-capture when the modal opens, not on every + // location change while it's open. + // eslint-disable-next-line react-hooks/exhaustive-deps + }, [isOpen]); + + useEffect(() => { + if (isOpen && location.pathname !== openedAtPathRef.current) { onOpenChange(false); } - }, [activeNavigation, isOpen, onOpenChange]); + }, [location, isOpen, onOpenChange]); const activeViewObj = useActiveView();