Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat(tabs): keep connection for tabs-[INS-4778] #8266

Open
wants to merge 12 commits into
base: feat/multiple-tab
Choose a base branch
from
Original file line number Diff line number Diff line change
Expand Up @@ -60,7 +60,7 @@ export const ResponseHistoryDropdown = ({

const handleDeleteResponses = useCallback(async () => {
if (isWebSocketResponse(activeResponse)) {
window.main.webSocket.closeAll();
window.main.webSocket.close({ requestId });
}
fetcher.submit({}, {
action: `/organization/${organizationId}/project/${projectId}/workspace/${workspaceId}/debug/request/${requestId}/response/delete-all`,
Expand Down
3 changes: 2 additions & 1 deletion packages/insomnia/src/ui/components/environment-picker.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ import { useFetcher, useNavigate, useParams, useRouteLoaderData } from 'react-ro

import { fuzzyMatch } from '../../common/misc';
import { isRemoteProject } from '../../models/project';
import uiEventBus from '../eventBus';
import { useOrganizationPermissions } from '../hooks/use-organization-features';
import type { WorkspaceLoaderData } from '../routes/workspace';
import { Icon } from './icon';
Expand Down Expand Up @@ -239,7 +240,6 @@ export const EnvironmentPicker = ({
return;
}
const [environmentId] = keys.values();

setActiveEnvironmentFetcher.submit(
{
environmentId,
Expand All @@ -249,6 +249,7 @@ export const EnvironmentPicker = ({
action: `/organization/${organizationId}/project/${projectId}/workspace/${workspaceId}/environment/set-active`,
}
);
uiEventBus.emit('CHANGE_ACTIVE_ENV', workspaceId);
}}
className="p-2 select-none text-sm overflow-y-auto focus:outline-none"
>
Expand Down
2 changes: 1 addition & 1 deletion packages/insomnia/src/ui/context/app/runner-context.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ import React, { createContext, type FC, type PropsWithChildren, useCallback, use
import type { Selection } from 'react-aria-components';

import type { UploadDataType } from '../../components/modals/upload-runner-data-modal';
import uiEventBus, { UIEventType } from '../../eventBus';
import uiEventBus from '../../eventBus';
import useStateRef from '../../hooks/use-state-ref';
import type { RequestRow } from '../../routes/runner';

Expand Down
4 changes: 2 additions & 2 deletions packages/insomnia/src/ui/eventBus.ts
Original file line number Diff line number Diff line change
@@ -1,10 +1,10 @@
type EventHandler = (...args: any[]) => void;

type UIEventType = 'CLOSE_TAB';

type UIEventType = 'CLOSE_TAB' | 'CHANGE_ACTIVE_ENV';
class EventBus {
private events: Record<UIEventType, EventHandler[]> = {
CLOSE_TAB: [],
CHANGE_ACTIVE_ENV: [],
};

// Subscribe to event
Expand Down
71 changes: 71 additions & 0 deletions packages/insomnia/src/ui/hooks/use-close-connection.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,71 @@
import { useCallback, useEffect } from 'react';

import * as models from '../../models';
import { isGrpcRequestId } from '../../models/grpc-request';
import { isEventStreamRequest, isGraphqlSubscriptionRequest, isRequestId } from '../../models/request';
import { isWebSocketRequestId } from '../../models/websocket-request';
import { useInsomniaTabContext } from '../context/app/insomnia-tab-context';
import uiEventBus from '../eventBus';

// this hook is use for control when to close connections(websocket & SSE & grpc stream & graphql subscription)
Copy link
Contributor

@jackkav jackkav Jan 8, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

a react hook is a mechanism to wrap react features into a observable function for reuse.

close all connections appears to be a fire and forget behavior.

why introduce all this complexity?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This scenario is indeed a bit complicated, we can't simply know when to close a connection, such as clicking a button and then closing the connection in the callback.
The connection may be closed because the user closed the tab manually or the user deleted the request or workspace. Using eventbus can decouple the connection control logic from the tab. Also I think abstract these logic into this custom hook could make the debug.tsx component simpler to read, and put the connection control logic together for easy maintenance.

export const useCloseConnection = ({ organizationId }: { organizationId: string }) => {

const closeConnectionById = async (id: string) => {
if (isGrpcRequestId(id)) {
window.main.grpc.cancel(id);
} else if (isWebSocketRequestId(id)) {
window.main.webSocket.close({ requestId: id });
} else if (isRequestId(id)) {
const request = await models.request.getById(id);
if (request && isEventStreamRequest(request)) {
window.main.curl.close({ requestId: id });
} else if (request && isGraphqlSubscriptionRequest(request)) {
window.main.webSocket.close({ requestId: id });
}
}
};

// close websocket&grpc&SSE connections
const handleTabClose = useCallback((_: string, ids: 'all' | string[]) => {
if (ids === 'all') {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If user just closse all tabs in one org, will this also close connections from other organizations?

window.main.webSocket.closeAll();
window.main.grpc.closeAll();
window.main.curl.closeAll();
return;
}

ids.forEach(async id => {
await closeConnectionById(id);
});
}, []);

const { currentOrgTabs } = useInsomniaTabContext();

const handleActiveEnvironmentChange = useCallback((workspaceId: string) => {
const { tabList } = currentOrgTabs;
const tabs = tabList.filter(tab => tab.workspaceId === workspaceId);
tabs.forEach(async tab => {
const id = tab.id;
await closeConnectionById(id);
});
}, [currentOrgTabs]);

useEffect(() => {
uiEventBus.on('CLOSE_TAB', handleTabClose);
uiEventBus.on('CHANGE_ACTIVE_ENV', handleActiveEnvironmentChange);

return () => {
uiEventBus.off('CLOSE_TAB', handleTabClose);
uiEventBus.off('CHANGE_ACTIVE_ENV', handleActiveEnvironmentChange);
};
}, [handleTabClose, handleActiveEnvironmentChange]);

// close all connections when organizationId change
useEffect(() => {
return () => {
window.main.webSocket.closeAll();
window.main.grpc.closeAll();
window.main.curl.closeAll();
};
}, [organizationId]);
};
12 changes: 5 additions & 7 deletions packages/insomnia/src/ui/routes/debug.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -105,6 +105,7 @@ import { getMethodShortHand } from '../components/tags/method-tag';
import { RealtimeResponsePane } from '../components/websockets/realtime-response-pane';
import { WebSocketRequestPane } from '../components/websockets/websocket-request-pane';
import { INSOMNIA_TAB_HEIGHT } from '../constant';
import { useCloseConnection } from '../hooks/use-close-connection';
import { useExecutionState } from '../hooks/use-execution-state';
import { useInsomniaTab } from '../hooks/use-insomnia-tab';
import { useReadyState } from '../hooks/use-ready-state';
Expand Down Expand Up @@ -452,13 +453,10 @@ export const Debug: FC = () => {
}
},
});
// Close all websocket connections when the active environment changes
useEffect(() => {
return () => {
window.main.webSocket.closeAll();
window.main.grpc.closeAll();
};
}, [activeEnvironment?._id]);
Comment on lines -455 to -461
Copy link
Member Author

@CurryYangxx CurryYangxx Dec 26, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This a trade-off; Previously, we closed all connections when the active environment changed to avoid users not knowing which URL is currently used to connect if included some env.
Now, based on multiple tabs, we may have multiple WebSocket or grpc tabs from different workspaces, and we want to keep all the connections when the corresponding tab exists. When a workspace's active environment changes, finding which requests are affected and closing them will introduce more complexity.
So I just delete it and only close connection when tab close.

Copy link
Member Author

@CurryYangxx CurryYangxx Jan 8, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

discuss with @ihexxa. I will try to emit an event through the event bus when modifying the active environment. So that we could get the workspaceId and filter the connections under the workspace then close them.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

updated in this commit: f367b65

Comment on lines -455 to -461
Copy link
Contributor

@jackkav jackkav Jan 8, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this use effect was clear about what happened and why, we close all connections when active env changes, the new hook is unclear about both of these questions.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Under the multiple-tabs scenario, when we switch to another tab that belongs to another workspace, activeEnvironment also changes and runs the logic in this useEffect. This is not as expected, we want to keep the connection when the tab exists, so we have to use another approach to implement it.


useCloseConnection({
organizationId,
});

const isRealtimeRequest =
activeRequest &&
Expand Down
Loading