From 692b86f85c7dd1b9f1ff8e3862e69d052f145fbc Mon Sep 17 00:00:00 2001 From: Sean Grove Date: Sun, 13 Feb 2022 22:37:56 -0800 Subject: [PATCH 1/4] feat: mark graph sessions as inactive so they don't show up in any UI --- npm-shrinkwrap.json | 14 ++-- package.json | 2 +- src/commands/dev/dev.js | 74 +++++++++++++----- src/commands/graph/graph-edit.js | 21 ++--- src/lib/one-graph/cli-client.js | 130 +++++++++++++++++++++++++++---- 5 files changed, 184 insertions(+), 57 deletions(-) diff --git a/npm-shrinkwrap.json b/npm-shrinkwrap.json index b50f5c1c28a..ce0afcf051d 100644 --- a/npm-shrinkwrap.json +++ b/npm-shrinkwrap.json @@ -82,7 +82,7 @@ "multiparty": "^4.2.1", "netlify": "^11.0.0", "netlify-headers-parser": "^6.0.1", - "netlify-onegraph-internal": "0.0.42", + "netlify-onegraph-internal": "0.0.50", "netlify-redirect-parser": "^13.0.2", "netlify-redirector": "^0.2.1", "node-fetch": "^2.6.0", @@ -16673,9 +16673,9 @@ } }, "node_modules/netlify-onegraph-internal": { - "version": "0.0.42", - "resolved": "https://registry.npmjs.org/netlify-onegraph-internal/-/netlify-onegraph-internal-0.0.42.tgz", - "integrity": "sha512-p9pxhXRmDtwxIcLE/H5iq+NfsjaoOQrBs00Ynac+0XWg3UoSsFenhTiY4lWBK3RbwpZqtkv+v4Z8eO+10YDr5w==", + "version": "0.0.50", + "resolved": "https://registry.npmjs.org/netlify-onegraph-internal/-/netlify-onegraph-internal-0.0.50.tgz", + "integrity": "sha512-58nsdeGU1a2OTR6UIdk6vd1HhIN3oj4X7EBX02inNK8O3Cx0zcMt1qGvsX6wbBkFMt/n2M8bUpJSwc3dPk3Jrg==", "dependencies": { "graphql": "16.0.0", "node-fetch": "^2.6.0", @@ -36036,9 +36036,9 @@ } }, "netlify-onegraph-internal": { - "version": "0.0.42", - "resolved": "https://registry.npmjs.org/netlify-onegraph-internal/-/netlify-onegraph-internal-0.0.42.tgz", - "integrity": "sha512-p9pxhXRmDtwxIcLE/H5iq+NfsjaoOQrBs00Ynac+0XWg3UoSsFenhTiY4lWBK3RbwpZqtkv+v4Z8eO+10YDr5w==", + "version": "0.0.50", + "resolved": "https://registry.npmjs.org/netlify-onegraph-internal/-/netlify-onegraph-internal-0.0.50.tgz", + "integrity": "sha512-58nsdeGU1a2OTR6UIdk6vd1HhIN3oj4X7EBX02inNK8O3Cx0zcMt1qGvsX6wbBkFMt/n2M8bUpJSwc3dPk3Jrg==", "requires": { "graphql": "16.0.0", "node-fetch": "^2.6.0", diff --git a/package.json b/package.json index 9ecd7796e75..bd4237203b5 100644 --- a/package.json +++ b/package.json @@ -270,7 +270,7 @@ "multiparty": "^4.2.1", "netlify": "^11.0.0", "netlify-headers-parser": "^6.0.1", - "netlify-onegraph-internal": "0.0.42", + "netlify-onegraph-internal": "0.0.50", "netlify-redirect-parser": "^13.0.2", "netlify-redirector": "^0.2.1", "node-fetch": "^2.6.0", diff --git a/src/commands/dev/dev.js b/src/commands/dev/dev.js index 9980d2bad0d..354dd243827 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,13 @@ const isNonExistingCommandError = ({ command, error: commandError }) => { ) } +/** + * @type {(() => Promise)[]} - array of functions to run before the process exits + */ +const cleanupWork = [] + +let cleanupStarted = false + /** * Run a command and pipe stdout, stderr and stdin * @param {string} command @@ -100,27 +108,47 @@ 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) - }) + // 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(' ') + // 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`) + } + }) + // eslint-disable-next-line promise/prefer-await-to-then + .then(() => { + // eslint-disable-next-line promise/always-return + if (!cleanupStarted) { + cleanupStarted = true + const cleanupFinished = Promise.all(cleanupWork.map((cleanup) => cleanup())) + return cleanupFinished + } + }) + // eslint-disable-next-line promise/prefer-await-to-then,promise/always-return + .then(() => { + process.exit(1) + }) ;['SIGINT', 'SIGTERM', 'SIGQUIT', 'SIGHUP', 'exit'].forEach((signal) => { - process.on(signal, () => { + process.on(signal, async () => { + if (!cleanupStarted) { + // eslint-disable-next-line no-unused-vars + const cleanupFinished = await Promise.all(cleanupWork.map((cleanup) => cleanup())) + cleanupStarted = true + } + commandProcess.kill('SIGTERM', { forceKillAfterTimeout: 500 }) process.exit() }) @@ -336,12 +364,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 884df88d776..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'] @@ -371,6 +395,7 @@ const upsertMergeCLISessionMetadata = async ({ netlifyToken, newMetadata, oneGra const detectedMetadata = detectLocalCLISessionMetadata({ siteRoot }) + // @ts-ignore const finalMetadata = { ...metadata, ...detectedMetadata, ...newMetadata } return OneGraphClient.updateCLISessionMetadata(netlifyToken, siteId, oneGraphSessionId, finalMetadata) } @@ -429,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) @@ -481,6 +500,7 @@ const startOneGraphCLISession = async (input) => { netlifyToken, netlifyGraphConfig, sessionId: oneGraphSessionId, + site, state, onEvents: async (events) => { for (const event of events) { @@ -500,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 @@ -511,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, @@ -522,10 +620,12 @@ const OneGraphCliClient = { module.exports = { OneGraphCliClient, createCLISession, + ensureCLISession, extractFunctionsFromOperationDoc, handleCliSessionEvent, generateSessionName, loadCLISession, + markCliSessionInactive, monitorCLISessionEvents, persistNewOperationsDocForSession, refetchAndGenerateFromOneGraph, From 01803346e3db100dd47b95c0c6fcf1e800c84033 Mon Sep 17 00:00:00 2001 From: Sean Grove Date: Mon, 14 Feb 2022 10:54:08 -0800 Subject: [PATCH 2/4] chore: centralize process.exit and cleanup logic --- src/commands/dev/dev.js | 42 ++++++++++++++++++----------------------- 1 file changed, 18 insertions(+), 24 deletions(-) diff --git a/src/commands/dev/dev.js b/src/commands/dev/dev.js index 354dd243827..10196a0eebb 100644 --- a/src/commands/dev/dev.js +++ b/src/commands/dev/dev.js @@ -86,6 +86,21 @@ 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 (cleanupStarted) { + // If cleanup has started, then wherever started it will be responsible for exiting + } else { + cleanupStarted = true + // eslint-disable-next-line no-unused-vars + const cleanupFinished = await Promise.all(cleanupWork.map((cleanup) => cleanup())) + process.exit(exitCode) + } +} + /** * Run a command and pipe stdout, stderr and stdin * @param {string} command @@ -114,7 +129,6 @@ const runCommand = (command, env = {}) => { .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, @@ -127,31 +141,11 @@ const runCommand = (command, env = {}) => { log(`${errorMessage}. Shutting down Netlify Dev server`) } - }) - // eslint-disable-next-line promise/prefer-await-to-then - .then(() => { - // eslint-disable-next-line promise/always-return - if (!cleanupStarted) { - cleanupStarted = true - const cleanupFinished = Promise.all(cleanupWork.map((cleanup) => cleanup())) - return cleanupFinished - } - }) - // eslint-disable-next-line promise/prefer-await-to-then,promise/always-return - .then(() => { - process.exit(1) - }) - ;['SIGINT', 'SIGTERM', 'SIGQUIT', 'SIGHUP', 'exit'].forEach((signal) => { - process.on(signal, async () => { - if (!cleanupStarted) { - // eslint-disable-next-line no-unused-vars - const cleanupFinished = await Promise.all(cleanupWork.map((cleanup) => cleanup())) - cleanupStarted = true - } - commandProcess.kill('SIGTERM', { forceKillAfterTimeout: 500 }) - process.exit() + return await cleanupBeforeExit({ exitCode: 1 }) }) + ;['SIGINT', 'SIGTERM', 'SIGQUIT', 'SIGHUP', 'exit'].forEach((signal) => { + process.on(signal, async () => await cleanupBeforeExit({})) }) return commandProcess From ded72f18d2be55b41ac8130f13979eb66a016680 Mon Sep 17 00:00:00 2001 From: Sean Grove Date: Mon, 14 Feb 2022 11:05:59 -0800 Subject: [PATCH 3/4] chore: address pr comments --- src/commands/dev/dev.js | 20 +++++++++++--------- 1 file changed, 11 insertions(+), 9 deletions(-) diff --git a/src/commands/dev/dev.js b/src/commands/dev/dev.js index 10196a0eebb..81bb869e5a5 100644 --- a/src/commands/dev/dev.js +++ b/src/commands/dev/dev.js @@ -91,13 +91,15 @@ let cleanupStarted = false * @param {number=} input.exitCode The exit code to return when exiting the process after cleanup */ const cleanupBeforeExit = async ({ exitCode }) => { - if (cleanupStarted) { - // If cleanup has started, then wherever started it will be responsible for exiting - } else { + // If cleanup has started, then wherever started it will be responsible for exiting + if (!cleanupStarted) { cleanupStarted = true - // eslint-disable-next-line no-unused-vars - const cleanupFinished = await Promise.all(cleanupWork.map((cleanup) => cleanup())) - process.exit(exitCode) + try { + // eslint-disable-next-line no-unused-vars + const cleanupFinished = await Promise.all(cleanupWork.map((cleanup) => cleanup())) + } finally { + process.exit(exitCode) + } } } @@ -144,9 +146,9 @@ const runCommand = (command, env = {}) => { return await cleanupBeforeExit({ exitCode: 1 }) }) - ;['SIGINT', 'SIGTERM', 'SIGQUIT', 'SIGHUP', 'exit'].forEach((signal) => { - process.on(signal, async () => await cleanupBeforeExit({})) - }) + ;['SIGINT', 'SIGTERM', 'SIGQUIT', 'SIGHUP', 'exit'].forEach((signal) => { + process.on(signal, async () => await cleanupBeforeExit({})) + }) return commandProcess } From 88cdb1b5588262324ec9c6eeeda5733fee4f778f Mon Sep 17 00:00:00 2001 From: Sean Grove Date: Mon, 14 Feb 2022 11:06:45 -0800 Subject: [PATCH 4/4] chore: fix formatting --- src/commands/dev/dev.js | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/commands/dev/dev.js b/src/commands/dev/dev.js index 81bb869e5a5..250e5fcc54b 100644 --- a/src/commands/dev/dev.js +++ b/src/commands/dev/dev.js @@ -146,9 +146,9 @@ const runCommand = (command, env = {}) => { return await cleanupBeforeExit({ exitCode: 1 }) }) - ;['SIGINT', 'SIGTERM', 'SIGQUIT', 'SIGHUP', 'exit'].forEach((signal) => { - process.on(signal, async () => await cleanupBeforeExit({})) - }) + ;['SIGINT', 'SIGTERM', 'SIGQUIT', 'SIGHUP', 'exit'].forEach((signal) => { + process.on(signal, async () => await cleanupBeforeExit({})) + }) return commandProcess }