mirror of
https://github.com/Kong/insomnia.git
synced 2026-11-06 04:53:59 -05:00
fix(project-modal): close on redirect by comparing location, not navigation.state
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 <noreply@anthropic.com>
This commit is contained in:
1 parent
66a777f863
commit
36ea5dce63
4 files changed
+22
-34
No files matched your search
@@ -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`);
|
||||
}
|
||||
@@ -87,23 +87,11 @@ const lockGenerator = () => {
|
||||
|
||||
const wrapWithLock = <T extends (...args: any[]) => Promise<any>>(fn: T): T => {
|
||||
const wrappedFn = async (...args: Parameters<T>): Promise<ReturnType<T>> => {
|
||||
// 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;
|
||||
|
||||
@@ -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;
|
||||
}
|
||||
|
||||
@@ -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<HTMLDivElement>(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();
|
||||
|
||||
|
||||
Reference in new issue
Block a user