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: persist proxy preferences #254

Merged
merged 31 commits into from
Oct 18, 2024
Merged
Show file tree
Hide file tree
Changes from 19 commits
Commits
Show all changes
31 commits
Select commit Hold shift + click to select a range
a3f98e5
feat: settings file and schema
cristianoventura Oct 10, 2024
15cd769
feat: allow proxy to run in different modes
cristianoventura Oct 10, 2024
79f83e0
feat: add base settings view and route
cristianoventura Oct 10, 2024
fbe16ad
feat: add proxy settings page
cristianoventura Oct 11, 2024
27e6aec
fix: upstream server validation
cristianoventura Oct 11, 2024
c3041c9
feat: show proxy status in settings page
cristianoventura Oct 11, 2024
003d90d
feat: ability to get current proxy status in ui
cristianoventura Oct 11, 2024
4a8abc9
feat: toast on save settings
cristianoventura Oct 11, 2024
a3bd445
feat: clean up proxy status
cristianoventura Oct 11, 2024
7fe6bbd
feat: add border to status indicator
cristianoventura Oct 11, 2024
89006cf
feat: loading state for settings form
cristianoventura Oct 11, 2024
27d969f
feat: add hint to port number field
cristianoventura Oct 11, 2024
d79b71c
feat: add support for authentication in upstream mode
cristianoventura Oct 11, 2024
d027731
refactor: extract proxy settings to a separate component
cristianoventura Oct 11, 2024
449af82
feat: add support to configure browser location
cristianoventura Oct 15, 2024
9f60d40
feat: add hint for proxy mode
cristianoventura Oct 15, 2024
151579c
fix: proxy emitter error when app is restored
cristianoventura Oct 15, 2024
c985e7e
fix: remove unused evevnt
cristianoventura Oct 15, 2024
7e7d80e
fix: reset form state when settings saved
cristianoventura Oct 15, 2024
3228c08
feat: improve UX with radio button and hints
cristianoventura Oct 16, 2024
8823896
Update src/components/Layout/ActivityBar/SettingsButton.tsx
cristianoventura Oct 16, 2024
04b6613
feat: schema version
cristianoventura Oct 16, 2024
d9b5c32
feat: hints on proxy mode
cristianoventura Oct 16, 2024
460ffda
Merge branch 'main' of github.com:grafana/k6-studio into feat/persist…
cristianoventura Oct 16, 2024
c1add15
feat: scrollarea in settings page
cristianoventura Oct 16, 2024
01836c4
refactor: extract types and rename variables
cristianoventura Oct 17, 2024
748ffdc
feat: dont save settings that are not being used
cristianoventura Oct 17, 2024
a26e00c
refactor: rename requireAuth
cristianoventura Oct 17, 2024
081503e
feat: settings dialog
cristianoventura Oct 17, 2024
b19d39b
feat: select browser executable dialog
cristianoventura Oct 17, 2024
f38dd73
fix: dirty form after file selection
cristianoventura Oct 17, 2024
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
2 changes: 2 additions & 0 deletions src/AppRoutes.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -13,11 +13,13 @@ import { RecordingPreviewer } from '@/views/RecordingPreviewer'
import { Generator } from '@/views/Generator/Generator'
import { Validator } from '@/views/Validator'
import { routeMap } from './routeMap'
import { Settings } from './views/Settings/Settings'

const router = createHashRouter(
createRoutesFromChildren(
<Route path={routeMap.home} element={<Layout />}>
<Route index element={<Home />} />
<Route path={routeMap.settings} element={<Settings />} />
<Route path={routeMap.recorder} element={<Recorder />} />
<Route
path={routeMap.recordingPreviewer}
Expand Down
22 changes: 16 additions & 6 deletions src/browser.ts
Original file line number Diff line number Diff line change
Expand Up @@ -9,17 +9,27 @@ import { getCertificateSPKI } from './proxy'
import { mkdtemp } from 'fs/promises'
import path from 'path'
import os from 'os'
import { proxyPort } from './main'
import { appSettings } from './main'

const createUserDataDir = async () => {
return mkdtemp(path.join(os.tmpdir(), 'k6-studio-'))
}

function getBrowserPath() {
const { recorder } = appSettings

if (recorder.detectBrowserPath) {
return computeSystemExecutablePath({
browser: Browser.CHROME,
channel: ChromeReleaseChannel.STABLE,
})
}

return recorder.browserPath as string
}

export const launchBrowser = async (browserWindow: BrowserWindow) => {
const path = computeSystemExecutablePath({
browser: Browser.CHROME,
channel: ChromeReleaseChannel.STABLE,
})
const path = getBrowserPath()
console.info(`browser path: ${path}`)

const userDataDir = await createUserDataDir()
Expand Down Expand Up @@ -54,7 +64,7 @@ export const launchBrowser = async (browserWindow: BrowserWindow) => {
'--disable-background-networking',
'--disable-component-update',
'--disable-search-engine-choice-screen',
`--proxy-server=http://localhost:${proxyPort}`,
`--proxy-server=http://localhost:${appSettings.proxy.port}`,
`--ignore-certificate-errors-spki-list=${certificateSPKI}`,
disableChromeOptimizations,
],
Expand Down
2 changes: 2 additions & 0 deletions src/components/Layout/ActivityBar/ActivityBar.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ import { VersionLabel } from './VersionLabel'
import { HomeIcon } from '@/components/icons'
import { NavIconButton } from './NavIconButton'
import { ApplicationLogButton } from './ApplicationLogButton'
import { SettingsButton } from './SettingsButton'

export function ActivityBar() {
return (
Expand Down Expand Up @@ -41,6 +42,7 @@ export function ActivityBar() {

<Flex direction="column" align="center" gap="3" mt="auto">
<ThemeSwitcher />
<SettingsButton />
<ApplicationLogButton />
<VersionLabel />
</Flex>
Expand Down
21 changes: 21 additions & 0 deletions src/components/Layout/ActivityBar/SettingsButton.tsx
Original file line number Diff line number Diff line change
@@ -0,0 +1,21 @@
import { getRoutePath } from '@/routeMap'
import { GearIcon } from '@radix-ui/react-icons'
import { Tooltip, IconButton } from '@radix-ui/themes'
import { useNavigate } from 'react-router-dom'

export function SettingsButton() {
const navigate = useNavigate()

return (
<Tooltip content="Settings" side="right">
<IconButton
are-label="Settings"
cristianoventura marked this conversation as resolved.
Show resolved Hide resolved
color="gray"
variant="ghost"
onClick={() => navigate(getRoutePath('settings'))}
>
<GearIcon />
</IconButton>
</Tooltip>
)
}
98 changes: 76 additions & 22 deletions src/main.ts
Original file line number Diff line number Diff line change
Expand Up @@ -45,6 +45,9 @@ import kill from 'tree-kill'
import find from 'find-process'
import { initializeLogger, openLogFolder } from './logger'
import log from 'electron-log/main'
import { AppSettings } from './schemas/appSettings'
import { getSettings, saveSettings } from './settings'
import { ProxyStatus } from './types'

// handle auto updates
if (process.env.NODE_ENV !== 'development') {
Expand All @@ -56,8 +59,8 @@ const proxyEmitter = new eventEmitter()
// Used mainly to avoid starting a new proxy when closing the active one on shutdown
let appShuttingDown: boolean = false
let currentProxyProcess: ProxyProcess | null
let proxyReady = false
export let proxyPort = 6000
let proxyStatus: ProxyStatus = 'offline'
export let appSettings: AppSettings
Comment on lines +62 to +63
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

  • Proxy status will be tracked across its life cycle so we can observe it in the UI.
  • All the app configuration now lives in appSettings. This variable is dynamic and will change as settings are changed.


let currentBrowserProcess: Process | null
let currentk6Process: K6Process | null
Expand Down Expand Up @@ -152,11 +155,19 @@ const createWindow = async () => {
}

mainWindow.once('ready-to-show', () => configureWatcher(mainWindow))
proxyEmitter.on('status:change', (statusName: ProxyStatus) => {
proxyStatus = statusName
mainWindow.webContents.send('proxy:status:change', statusName)
})
Comment on lines +158 to +161
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Since we already have an EventEmitter for the proxy I used it as a wrapper for changes related to the proxy status.

mainWindow.on('closed', () =>
proxyEmitter.removeAllListeners('status:change')
)

return mainWindow
}

app.whenReady().then(async () => {
appSettings = await getSettings()
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Retrieving settings is the first event when the app launches. getSettings is an upsert so it'll create a configuration file if none is found.

await createSplashWindow()
await setupProjectStructure()
await createWindow()
Expand Down Expand Up @@ -188,11 +199,11 @@ app.on('before-quit', async () => {
})

// Proxy
ipcMain.handle('proxy:start', async (event, port?: number) => {
ipcMain.handle('proxy:start', async (event) => {
console.info('proxy:start event received')

const browserWindow = browserWindowFromEvent(event)
currentProxyProcess = await launchProxyAndAttachEmitter(browserWindow, port)
currentProxyProcess = await launchProxyAndAttachEmitter(browserWindow)
})

ipcMain.on('proxy:stop', async () => {
Expand All @@ -201,7 +212,7 @@ ipcMain.on('proxy:stop', async () => {
})

const waitForProxy = async (): Promise<void> => {
if (proxyReady) {
if (proxyStatus === 'online') {
return Promise.resolve()
}

Expand Down Expand Up @@ -292,7 +303,7 @@ ipcMain.handle(
currentk6Process = await runScript(
browserWindow,
resolvedScriptPath,
proxyPort
appSettings.proxy.port
)
}
)
Expand Down Expand Up @@ -506,6 +517,54 @@ ipcMain.handle('app:open-log', () => {
openLogFolder()
})

ipcMain.handle('settings:get', async () => {
console.info('settings:get event received')
return await getSettings()
})
Comment on lines +520 to +523
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Event handler to retrieve settings in the UI.


ipcMain.handle('settings:save', async (event, data: AppSettings) => {
console.info('settings:save event received')

const browserWindow = browserWindowFromEvent(event)
try {
const diff = await saveSettings(data)
applySettings(diff, browserWindow)
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Event handler for saving settings from the UI. saveSettings returns the diff with only settings that have been modified. applySettings is responsible for overwriting and performing specific changes.

Copy link
Member

Choose a reason for hiding this comment

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

minor nit: might make more sense to call it diff -> modifiedSettings 🤔


sendToast(browserWindow.webContents, {
title: 'Settings saved successfully',
status: 'success',
})
return true
} catch (error) {
log.error(error)
sendToast(browserWindow.webContents, {
title: 'Failed to save settings',
status: 'error',
})
return false
}
})

ipcMain.handle('proxy:status:get', async () => {
console.info('proxy:status:get event received')
return proxyStatus
})

async function applySettings(
diff: Partial<AppSettings>,
browserWindow: BrowserWindow
) {
if (diff.proxy) {
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

This is where configuration changes when settings are saved.

Copy link
Member

Choose a reason for hiding this comment

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

same as above, renaming diff might make it more explicit/readable!

stopProxyProcess()
appSettings.proxy = diff.proxy
proxyEmitter.emit('status:change', 'restarting')
currentProxyProcess = await launchProxyAndAttachEmitter(browserWindow)
}
if (diff.recorder) {
appSettings.recorder = diff.recorder
}
}

const browserWindowFromEvent = (
event: Electron.IpcMainEvent | Electron.IpcMainInvokeEvent
) => {
Expand All @@ -518,29 +577,25 @@ const browserWindowFromEvent = (
return browserWindow
}

const launchProxyAndAttachEmitter = async (
browserWindow: BrowserWindow,
port: number = proxyPort
) => {
// confirm that the port is still open and if not get the next open one
const availableOpenport = await findOpenPort(port)
console.log(`proxy open port found: ${availableOpenport}`)
const launchProxyAndAttachEmitter = async (browserWindow: BrowserWindow) => {
const { port, findPort } = appSettings.proxy

if (availableOpenport !== proxyPort) {
proxyPort = availableOpenport
}
const proxyPort = findPort ? await findOpenPort(port) : port
appSettings.proxy.port = proxyPort

console.log(`launching proxy ${JSON.stringify(appSettings.proxy)}`)

return launchProxy(browserWindow, proxyPort, {
return launchProxy(browserWindow, appSettings.proxy, {
onReady: () => {
proxyReady = true
proxyEmitter.emit('status:change', 'online')
proxyEmitter.emit('ready')
},
onFailure: async () => {
if (appShuttingDown) {
// we don't have to restart the proxy if the app is shutting down
if (appShuttingDown || proxyStatus === 'restarting') {
// don't restart the proxy if the app is shutting down or if it's already restarting
return
}
proxyReady = false
proxyEmitter.emit('status:change', 'restarting')
currentProxyProcess = await launchProxyAndAttachEmitter(browserWindow)

sendToast(browserWindow.webContents, {
Expand Down Expand Up @@ -593,7 +648,6 @@ const stopProxyProcess = () => {
// NOTE: this might not kill the second spawned process on windows
currentProxyProcess.kill()
currentProxyProcess = null
proxyReady = false
}
}

Expand Down
19 changes: 18 additions & 1 deletion src/preload.ts
Original file line number Diff line number Diff line change
@@ -1,8 +1,9 @@
import { ipcRenderer, contextBridge, IpcRendererEvent } from 'electron'
import { ProxyData, K6Log, FolderContent, K6Check } from './types'
import { ProxyData, K6Log, FolderContent, K6Check, ProxyStatus } from './types'
import { HarFile } from './types/har'
import { GeneratorFile } from './types/generator'
import { AddToastPayload } from './types/toast'
import { AppSettings } from './schemas/appSettings'

// Create listener and return clean up function to be used in useEffect
function createListener<T>(channel: string, callback: (data: T) => void) {
Expand All @@ -27,6 +28,12 @@ const proxy = {
onProxyData: (callback: (data: ProxyData) => void) => {
return createListener('proxy:data', callback)
},
getProxyStatus: () => {
return ipcRenderer.invoke('proxy:status:get')
},
onProxyStatusChange: (callback: (status: ProxyStatus) => void) => {
return createListener('proxy:status:change', callback)
},
} as const

const browser = {
Expand Down Expand Up @@ -147,6 +154,15 @@ const app = {
},
} as const

const settings = {
getSettings: () => {
return ipcRenderer.invoke('settings:get')
},
saveSettings: (settings: AppSettings): Promise<AppSettings> => {
return ipcRenderer.invoke('settings:save', settings)
},
}

const studio = {
proxy,
browser,
Expand All @@ -155,6 +171,7 @@ const studio = {
ui,
generator,
app,
settings,
} as const

contextBridge.exposeInMainWorld('studio', studio)
Expand Down
26 changes: 24 additions & 2 deletions src/proxy.ts
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ import { ProxyData } from './types'
import readline from 'readline/promises'
import { safeJsonParse } from './utils/json'
import log from 'electron-log/main'
import { ProxySettings } from './schemas/appSettings'

export type ProxyProcess = ChildProcessWithoutNullStreams

Expand All @@ -18,7 +19,7 @@ interface options {

export const launchProxy = (
browserWindow: BrowserWindow,
port?: number,
proxySettings: ProxySettings,
{ onReady, onFailure }: options = {}
): ProxyProcess => {
let proxyScript: string
Expand Down Expand Up @@ -50,10 +51,17 @@ export const launchProxy = (
proxyScript,
'--set',
`confdir=${certificatesPath}`,
'--listen-port',
`${proxySettings.port}`,
Comment on lines +54 to +55
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

The proxy should always listen on the port from the settings file.

'--mode',
`regular@${port}`,
getProxyMode(proxySettings),
]

if (proxyRequiresAuth(proxySettings)) {
const { username, password } = proxySettings.upstream
proxyArgs.push('--upstream-auth', `${username}:${password}`)
}
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Appends authentication argument when upstream mode requires auth.


const proxy = spawn(proxyPath, proxyArgs)

// we use a reader to read entire lines from stdout instead of buffered data
Expand Down Expand Up @@ -94,6 +102,20 @@ export const launchProxy = (
return proxy
}

const getProxyMode = (proxySettings: ProxySettings) => {
const { mode, upstream } = proxySettings

if (mode === 'upstream') {
return `upstream:${upstream.url}`
}

return 'regular'
}

const proxyRequiresAuth = (proxySettings: ProxySettings) => {
return proxySettings.mode === 'upstream' && proxySettings.upstream.requireAuth
}

export const getCertificatesPath = () => {
if (MAIN_WINDOW_VITE_DEV_SERVER_URL) {
return path.join(app.getAppPath(), 'resources', 'certificates')
Expand Down
2 changes: 2 additions & 0 deletions src/routeMap.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ import { generatePath } from 'react-router-dom'
const routes = {
home: '/',
recorder: '/recorder',
settings: '/settings',
recordingPreviewer: '/recording-previewer/:fileName',
validator: '/validator/:fileName?',
generator: '/generator/:fileName',
Expand All @@ -27,4 +28,5 @@ export const routeMap = {
recordingPreviewer: getRoutePath('recordingPreviewer'),
generator: getRoutePath('generator'),
validator: getRoutePath('validator'),
settings: getRoutePath('settings'),
}
Loading