From 2913759b8663542124fcefd447dfb8cb5d1042fa Mon Sep 17 00:00:00 2001 From: Sean Grove Date: Mon, 14 Feb 2022 11:46:31 -0800 Subject: [PATCH] feat: mark graph sessions as inactive so they don't show up in any UI (#4285) * feat: mark graph sessions as inactive so they don't show up in any UI * chore: centralize process.exit and cleanup logic * chore: address pr comments * chore: fix formatting Co-authored-by: kodiakhq[bot] <49736102+kodiakhq[bot]@users.noreply.github.com> --- src/commands/dev/dev.js | 76 ++++++++++++------ src/commands/graph/graph-edit.js | 21 ++--- src/lib/one-graph/cli-client.js | 129 +++++++++++++++++++++++++++---- 3 files changed, 174 insertions(+), 52 deletions(-) diff --git a/src/commands/dev/dev.js b/src/commands/dev/dev.js index 9980d2bad0d..250e5fcc54b 100644 --- a/src/commands/dev/dev.js +++ b/src/commands/dev/dev.js @@ -14,6 +14,7 @@ const { startFunctionsServer } = require('../../lib/functions/server') const { OneGraphCliClient, loadCLISession, + markCliSessionInactive, persistNewOperationsDocForSession, startOneGraphCLISession, } = require('../../lib/one-graph/cli-client') @@ -78,6 +79,30 @@ const isNonExistingCommandError = ({ command, error: commandError }) => { ) } +/** + * @type {(() => Promise)[]} - array of functions to run before the process exits + */ +const cleanupWork = [] + +let cleanupStarted = false + +/** + * @param {object} input + * @param {number=} input.exitCode The exit code to return when exiting the process after cleanup + */ +const cleanupBeforeExit = async ({ exitCode }) => { + // If cleanup has started, then wherever started it will be responsible for exiting + if (!cleanupStarted) { + cleanupStarted = true + try { + // eslint-disable-next-line no-unused-vars + const cleanupFinished = await Promise.all(cleanupWork.map((cleanup) => cleanup())) + } finally { + process.exit(exitCode) + } + } +} + /** * Run a command and pipe stdout, stderr and stdin * @param {string} command @@ -100,30 +125,29 @@ const runCommand = (command, env = {}) => { // we can't try->await->catch since we don't want to block on the framework server which // is a long running process - // eslint-disable-next-line promise/catch-or-return,promise/prefer-await-to-then - commandProcess.then(async () => { - const result = await commandProcess - const [commandWithoutArgs] = command.split(' ') - // eslint-disable-next-line promise/always-return - if (result.failed && isNonExistingCommandError({ command: commandWithoutArgs, error: result })) { - log( - NETLIFYDEVERR, - `Failed running command: ${command}. Please verify ${chalk.magenta(`'${commandWithoutArgs}'`)} exists`, - ) - } else { - const errorMessage = result.failed - ? `${NETLIFYDEVERR} ${result.shortMessage}` - : `${NETLIFYDEVWARN} "${command}" exited with code ${result.exitCode}` - - log(`${errorMessage}. Shutting down Netlify Dev server`) - } - process.exit(1) - }) - ;['SIGINT', 'SIGTERM', 'SIGQUIT', 'SIGHUP', 'exit'].forEach((signal) => { - process.on(signal, () => { - commandProcess.kill('SIGTERM', { forceKillAfterTimeout: 500 }) - process.exit() + // eslint-disable-next-line promise/catch-or-return + commandProcess + // eslint-disable-next-line promise/prefer-await-to-then + .then(async () => { + const result = await commandProcess + const [commandWithoutArgs] = command.split(' ') + if (result.failed && isNonExistingCommandError({ command: commandWithoutArgs, error: result })) { + log( + NETLIFYDEVERR, + `Failed running command: ${command}. Please verify ${chalk.magenta(`'${commandWithoutArgs}'`)} exists`, + ) + } else { + const errorMessage = result.failed + ? `${NETLIFYDEVERR} ${result.shortMessage}` + : `${NETLIFYDEVWARN} "${command}" exited with code ${result.exitCode}` + + log(`${errorMessage}. Shutting down Netlify Dev server`) + } + + return await cleanupBeforeExit({ exitCode: 1 }) }) + ;['SIGINT', 'SIGTERM', 'SIGQUIT', 'SIGHUP', 'exit'].forEach((signal) => { + process.on(signal, async () => await cleanupBeforeExit({})) }) return commandProcess @@ -336,12 +360,18 @@ const dev = async (options, command) => { const oneGraphSessionId = loadCLISession(state) await persistNewOperationsDocForSession({ + netlifyGraphConfig, netlifyToken, oneGraphSessionId, operationsDoc: graphqlDocument, siteId: site.id, + siteRoot: site.root, }) + const cleanupSession = () => markCliSessionInactive({ netlifyToken, sessionId: oneGraphSessionId, siteId: site.id }) + + cleanupWork.push(cleanupSession) + const graphEditUrl = getGraphEditUrlBySiteId({ siteId: site.id, oneGraphSessionId }) log( diff --git a/src/commands/graph/graph-edit.js b/src/commands/graph/graph-edit.js index 9b247e8ab79..f07dbc1d759 100644 --- a/src/commands/graph/graph-edit.js +++ b/src/commands/graph/graph-edit.js @@ -1,13 +1,7 @@ // @ts-check const gitRepoInfo = require('git-repo-info') -const { - OneGraphCliClient, - createCLISession, - generateSessionName, - loadCLISession, - upsertMergeCLISessionMetadata, -} = require('../../lib/one-graph/cli-client') +const { OneGraphCliClient, ensureCLISession, upsertMergeCLISessionMetadata } = require('../../lib/one-graph/cli-client') const { defaultExampleOperationsDoc, getGraphEditUrlBySiteId, @@ -48,13 +42,12 @@ const graphEdit = async (options, command) => { await ensureAppForSite(netlifyToken, siteId) - let oneGraphSessionId = loadCLISession(state) - if (!oneGraphSessionId) { - const sessionName = generateSessionName() - const oneGraphSession = await createCLISession({ netlifyToken, siteId: site.id, sessionName, metadata: {} }) - state.set('oneGraphSessionId', oneGraphSession.id) - oneGraphSessionId = state.get('oneGraphSessionId') - } + const oneGraphSessionId = await ensureCLISession({ + metadata: {}, + netlifyToken, + site, + state, + }) const { branch } = gitRepoInfo() const persistedDoc = await createPersistedQuery(netlifyToken, { diff --git a/src/lib/one-graph/cli-client.js b/src/lib/one-graph/cli-client.js index 22d8dc7224b..331375656f7 100644 --- a/src/lib/one-graph/cli-client.js +++ b/src/lib/one-graph/cli-client.js @@ -24,7 +24,13 @@ const { const { parse } = GraphQL const { defaultExampleOperationsDoc, extractFunctionsFromOperationDoc } = NetlifyGraph -const { createPersistedQuery, ensureAppForSite, updateCLISessionMetadata } = OneGraphClient +const { + createPersistedQuery, + ensureAppForSite, + executeMarkCliSessionActiveHeartbeat, + executeMarkCliSessionInactive, + updateCLISessionMetadata, +} = OneGraphClient const internalConsole = { log, @@ -51,13 +57,31 @@ InternalConsole.registerConsole(internalConsole) * @param {function} input.onEvents A function to call when CLI events are received and need to be processed * @param {string} input.sessionId The session id to monitor CLI events for * @param {StateConfig} input.state A function to call to set/get the current state of the local Netlify project + * @param {any} input.site The site object * @returns */ const monitorCLISessionEvents = (input) => { - const { appId, netlifyGraphConfig, netlifyToken, onClose, onError, onEvents, sessionId, state } = input + const { appId, netlifyGraphConfig, netlifyToken, onClose, onError, onEvents, sessionId, site, state } = input const frequency = 5000 + // 30 minutes + const defaultHeartbeatFrequency = 1_800_000 let shouldClose = false + let nextMarkActiveHeartbeat = defaultHeartbeatFrequency + + const markActiveHelper = async () => { + const fullSession = await OneGraphClient.fetchCliSession({ authToken: netlifyToken, appId, sessionId }) + // @ts-ignore + const heartbeatIntervalms = fullSession.session.cliHeartbeatIntervalMs || defaultHeartbeatFrequency + nextMarkActiveHeartbeat = heartbeatIntervalms + const markCLISessionActiveResult = await executeMarkCliSessionActiveHeartbeat(netlifyToken, site.id, sessionId) + if (markCLISessionActiveResult.errors && markCLISessionActiveResult.errors.length !== 0) { + warn(`Failed to mark CLI session active: ${markCLISessionActiveResult.errors.join(', ')}`) + } + setTimeout(markActiveHelper, nextMarkActiveHeartbeat) + } + + setTimeout(markActiveHelper, nextMarkActiveHeartbeat) const enabledServiceWatcher = async (innerNetlifyToken, siteId) => { const enabledServices = state.get('oneGraphEnabledServices') || ['onegraph'] @@ -430,19 +454,13 @@ const loadCLISession = (state) => state.get('oneGraphSessionId') const startOneGraphCLISession = async (input) => { const { netlifyGraphConfig, netlifyToken, site, state } = input OneGraphClient.ensureAppForSite(netlifyToken, site.id) - let oneGraphSessionId = loadCLISession(state) - if (!oneGraphSessionId) { - const sessionName = generateSessionName() - const sessionMetadata = {} - const oneGraphSession = await createCLISession({ - netlifyToken, - siteId: site.id, - sessionName, - metadata: sessionMetadata, - }) - state.set('oneGraphSessionId', oneGraphSession.id) - oneGraphSessionId = state.get('oneGraphSessionId') - } + + const oneGraphSessionId = await ensureCLISession({ + metadata: {}, + netlifyToken, + site, + state, + }) const enabledServices = [] const schema = await OneGraphClient.fetchOneGraphSchema(site.id, enabledServices) @@ -482,6 +500,7 @@ const startOneGraphCLISession = async (input) => { netlifyToken, netlifyGraphConfig, sessionId: oneGraphSessionId, + site, state, onEvents: async (events) => { for (const event of events) { @@ -501,6 +520,20 @@ const startOneGraphCLISession = async (input) => { }) } +/** + * Mark a session as inactive so it doesn't show up in any UI lists, and potentially becomes available to GC later + * @param {object} input + * @param {string} input.netlifyToken The (typically netlify) access token that is used for authentication, if any + * @param {string} input.siteId A function to call to set/get the current state of the local Netlify project + * @param {string} input.sessionId The session id to monitor CLI events for + */ +const markCliSessionInactive = async ({ netlifyToken, sessionId, siteId }) => { + const result = await executeMarkCliSessionInactive(netlifyToken, siteId, sessionId) + if (result.errors) { + warn(`Unable to mark CLI session ${sessionId} inactive: ${JSON.stringify(result.errors, null, 2)}`) + } +} + /** * Generate a session name that can be identified as belonging to the current checkout * @returns {string} The name of the session to create @@ -512,6 +545,70 @@ const generateSessionName = () => { return sessionName } +/** + * Ensures a cli session exists for the current checkout, or errors out if it doesn't and cannot create one. + */ +const ensureCLISession = async ({ metadata, netlifyToken, site, state }) => { + let oneGraphSessionId = loadCLISession(state) + let parentCliSessionId = null + + // Validate that session still exists and we can access it + try { + if (oneGraphSessionId) { + const sessionEvents = await OneGraphClient.fetchCliSessionEvents({ + appId: site.id, + authToken: netlifyToken, + sessionId: oneGraphSessionId, + }) + if (sessionEvents.errors) { + warn(`Unable to fetch cli session: ${JSON.stringify(sessionEvents.errors, null, 2)}`) + log(`Creating new cli session`) + parentCliSessionId = oneGraphSessionId + oneGraphSessionId = null + } + } + } catch (fetchSessionError) { + warn(`Unable to fetch cli session events: ${JSON.stringify(fetchSessionError, null, 2)}`) + oneGraphSessionId = null + } + + if (!oneGraphSessionId) { + // If we can't access the session in the state.json or it doesn't exist, create a new one + const sessionName = generateSessionName() + const detectedMetadata = detectLocalCLISessionMetadata({ siteRoot: site.root }) + const newSessionMetadata = parentCliSessionId ? { parentCliSessionId } : {} + const sessionMetadata = { + ...detectedMetadata, + ...newSessionMetadata, + ...metadata, + } + const oneGraphSession = await createCLISession({ + netlifyToken, + siteId: site.id, + sessionName, + metadata: sessionMetadata, + }) + state.set('oneGraphSessionId', oneGraphSession.id) + oneGraphSessionId = state.get('oneGraphSessionId') + } + + if (!oneGraphSessionId) { + error('Unable to create or access Netlify Graph CLI session') + } + + const { errors: markCLISessionActiveErrors } = await executeMarkCliSessionActiveHeartbeat( + netlifyToken, + site.id, + oneGraphSessionId, + ) + + if (markCLISessionActiveErrors) { + warn(`Unable to mark cli session active: ${JSON.stringify(markCLISessionActiveErrors, null, 2)}`) + } + + return oneGraphSessionId +} + const OneGraphCliClient = { ackCLISessionEvents: OneGraphClient.ackCLISessionEvents, createPersistedQuery, @@ -523,10 +620,12 @@ const OneGraphCliClient = { module.exports = { OneGraphCliClient, createCLISession, + ensureCLISession, extractFunctionsFromOperationDoc, handleCliSessionEvent, generateSessionName, loadCLISession, + markCliSessionInactive, monitorCLISessionEvents, persistNewOperationsDocForSession, refetchAndGenerateFromOneGraph,