Skip to content

Commit

Permalink
feat: mark graph sessions as inactive so they don't show up in any UI (
Browse files Browse the repository at this point in the history
…#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>
  • Loading branch information
sgrove and kodiakhq[bot] authored Feb 14, 2022
1 parent 050bf90 commit 2913759
Show file tree
Hide file tree
Showing 3 changed files with 174 additions and 52 deletions.
76 changes: 53 additions & 23 deletions src/commands/dev/dev.js
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@ const { startFunctionsServer } = require('../../lib/functions/server')
const {
OneGraphCliClient,
loadCLISession,
markCliSessionInactive,
persistNewOperationsDocForSession,
startOneGraphCLISession,
} = require('../../lib/one-graph/cli-client')
Expand Down Expand Up @@ -78,6 +79,30 @@ const isNonExistingCommandError = ({ command, error: commandError }) => {
)
}

/**
* @type {(() => Promise<void>)[]} - 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
Expand All @@ -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
Expand Down Expand Up @@ -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(
Expand Down
21 changes: 7 additions & 14 deletions src/commands/graph/graph-edit.js
Original file line number Diff line number Diff line change
@@ -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,
Expand Down Expand Up @@ -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, {
Expand Down
129 changes: 114 additions & 15 deletions src/lib/one-graph/cli-client.js
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand All @@ -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']
Expand Down Expand Up @@ -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)
Expand Down Expand Up @@ -482,6 +500,7 @@ const startOneGraphCLISession = async (input) => {
netlifyToken,
netlifyGraphConfig,
sessionId: oneGraphSessionId,
site,
state,
onEvents: async (events) => {
for (const event of events) {
Expand All @@ -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
Expand All @@ -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,
Expand All @@ -523,10 +620,12 @@ const OneGraphCliClient = {
module.exports = {
OneGraphCliClient,
createCLISession,
ensureCLISession,
extractFunctionsFromOperationDoc,
handleCliSessionEvent,
generateSessionName,
loadCLISession,
markCliSessionInactive,
monitorCLISessionEvents,
persistNewOperationsDocForSession,
refetchAndGenerateFromOneGraph,
Expand Down

1 comment on commit 2913759

@github-actions
Copy link

Choose a reason for hiding this comment

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

📊 Benchmark results

Package size: 442 MB

Please sign in to comment.