Skip to content
This repository was archived by the owner on Jul 9, 2025. It is now read-only.

fix: Added cleanup logic to app on quit #7645

Merged
tonyanziano merged 9 commits into from
May 5, 2021
Merged

fix: Added cleanup logic to app on quit #7645

Show file tree
Hide file tree
Changes from 3 commits
Commits
Show all changes
9 commits
Select commit Hold shift + click to select a range
a4668e6
Added cleanup logic to app on quit
tonyanziano May 5, 2021
3daa922
Merge branch 'main' into toanzian/app-cleanup
tonyanziano May 5, 2021
17dbe04
Merge branch 'main' into toanzian/app-cleanup
tonyanziano May 5, 2021
bb97a55
Added missing electron ipc event listener
tonyanziano May 5, 2021
1ce8939
Merge branch 'toanzian/app-cleanup' of https://github.com/microsoft/B…
tonyanziano May 5, 2021
f9520e0
Merge branch 'main' into toanzian/app-cleanup
tonyanziano May 5, 2021
fbdf30d
Merge branch 'main' into toanzian/app-cleanup
tdurnford May 5, 2021
64a7727
Merge branch 'main' into toanzian/app-cleanup
hatpick May 5, 2021
87d06b4
Merge branch 'main' into toanzian/app-cleanup
tonyanziano May 5, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 4 additions & 1 deletion Composer/packages/client/src/App.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,9 @@ const Logger = () => {
export const App: React.FC = () => {
const { appLocale } = useRecoilValue(userSettingsState);

const { fetchExtensions, fetchFeatureFlags, checkNodeVersion } = useRecoilValue(dispatcherState);
const { fetchExtensions, fetchFeatureFlags, checkNodeVersion, performAppCleanupOnQuit } = useRecoilValue(
dispatcherState
);

useEffect(() => {
loadLocale(appLocale);
Expand All @@ -32,6 +34,7 @@ export const App: React.FC = () => {
checkNodeVersion();
fetchExtensions();
fetchFeatureFlags();
performAppCleanupOnQuit();

@tdurnford tdurnford May 5, 2021

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Shouldn't this be invoke when electron sends the 'cleanup' event? How is this supposed to be triggered when the user is running it in the browser?

const { ipcRenderer } = window;

useEffect(() => {
  ipcRenderer?.on('cleanup', (_event) => {
      performAppCleanupOnQuit();
  });
}, []);

@tonyanziano tonyanziano May 5, 2021 •
edited
Loading

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

It is essentially a no-op for the browser.

My understanding is that since the server (Node process) that runs the app in the browser is responsible for spawning the bot processes, the processes should shut down when the user stops the server. The user exiting the browser tab shouldn't necessarily stop the server, where as in Electron, exiting the client also shuts down the server.

@tonyanziano tonyanziano May 5, 2021

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Oh I see now. I was moving files around and I forgot to add the ipc code that I previously had. Yes, it should have a listener like that.

}, []);

return (
Expand Down
7 changes: 7 additions & 0 deletions Composer/packages/client/src/recoilModel/dispatchers/application.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@ import { DebugDrawerKeys } from '../../pages/design/DebugPanel/TabExtensions/typ
import httpClient from '../../utils/httpUtil';

import { setError } from './shared';
import { flushExistingTasks } from './utils/project';

export const applicationDispatcher = () => {
const setAppUpdateStatus = useRecoilCallback(
Expand Down Expand Up @@ -147,6 +148,11 @@ export const applicationDispatcher = () => {
}
});

const performAppCleanupOnQuit = useRecoilCallback((callbackHelpers: CallbackInterface) => async () => {
// shutdown any running bots to avoid orphaned processes
await flushExistingTasks(callbackHelpers);
});

return {
checkNodeVersion,
setAppUpdateStatus,
Expand All @@ -156,6 +162,7 @@ export const applicationDispatcher = () => {
setMessage: debounce(setMessage, 500),
onboardingSetComplete,
onboardingAddCoachMarkRef,
performAppCleanupOnQuit,
setCreationFlowStatus,
setApplicationLevelError,
setCreationFlowType,
Expand Down
5 changes: 2 additions & 3 deletions Composer/packages/client/src/utils/electronUtil.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,10 +2,8 @@
// Licensed under the MIT License.
import { PublishTarget } from '@bfc/shared';

// import { armScopes } from '../constants';

import httpClient from './httpUtil';
// import { AuthClient } from './authClient';

/**
* Returns true if the client is embedded in the Composer Electron environment.
*/
Expand All @@ -23,6 +21,7 @@ type ABSProfile = {
armEndpoint?: string;
};
export type Profile = ABSProfile; // can include PVAProfile or other type of profile by |

/**
*
* @param profile payload from bf's create protocol
Expand Down
1 change: 1 addition & 0 deletions Composer/packages/electron-server/src/main.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -311,6 +311,7 @@ async function run() {
app.on('before-quit', () => {
const mainWindow = ElectronWindow.getInstance().browserWindow;
mainWindow?.webContents.send('session-update', 'session-ended');
mainWindow?.webContents.send('cleanup');
});

app.on('activate', () => {
Expand Down