fix: admin/config: sometimes busy-port message was including process name because of timing

Massimo Melina committed Jun 3, 2022 at 16:38 UTC 21b14ec800f619b1eb2d113ccbe3c882f8e7017e
4 files changed +21 -15
admin/src/ConfigPage.ts
+2 -4
@@ -28,10 +28,8 @@ export default function ConfigPage() {
28 let snap = useSnapState()
29 const statusApi = useApiEx(data && 'get_status')
30 const status = statusApi.data
31 - const reloadStatus = statusApi.reload
31 + const reloadStatus = exposedReloadStatus = statusApi.reload
32 useEffect(reloadStatus, [data]) //eslint-disable-line
33 -
34 - exposedReloadStatus = reloadStatus
33 useEffect(() => () => exposedReloadStatus = undefined, []) // clear on unmount
34
35 const admins = useApi('get_admins')[0]?.list
@@ -141,7 +139,7 @@ export default function ConfigPage() {
139 await alertDialog("You are being redirected but in some cases this may fail. Hold on tight!", 'warning')
140 return window.location.href = loc.protocol + '//' + loc.hostname + ':' + newPort + loc.pathname
141 }
144 - setTimeout(reloadStatus, 2000) // in case of busy port, finding the name of the process can be a lengthy task. 2s is hopefully enough. Worst case we'll kee the generic error message
142 + setTimeout(reloadStatus, 'port' in values || 'https_port' in values ? 1000 : 0) // give some time to consider new ports
143 Object.assign(loaded, values) // since changes are recalculated subscribing state.config, but it depends on 'loaded' to (which cannot be subscribed), be sure to update loaded first
144 recalculateChanges()
145 toast("Changes applied", 'success')
admin/src/api.ts
+6 -2
@@ -33,13 +33,17 @@ export function useApiEx<T=any>(...args: Parameters<typeof useApi>) {
33
34 const PREFIX = '/~/api/'
35
36 -export function apiCall(cmd: string, params?: Dict) : Promise<any> {
36 +const timeoutByApi: Dict = {
37 + get_status: 20 // can be lengthy on slow machines because of the find-process-on-busy-port feature
38 +}
39 +export function apiCall(cmd: string, params?: Dict, { timeout=undefined }={}) : Promise<any> {
40 const csrf = getCsrf()
41 if (csrf)
42 params = { csrf, ...params }
43
44 const controller = new AbortController()
42 - setTimeout(() => controller.abort(), 10_000)
45 + if (timeout !== false)
46 + setTimeout(() => controller.abort(), (timeoutByApi[cmd] ?? timeout ?? 10)*1000)
47 return fetch(PREFIX+cmd, {
48 method: 'POST',
49 headers: { 'content-type': 'application/json' },
server/src/adminApis.ts
+8 -5
@@ -9,7 +9,7 @@ import accountsApis from './api.accounts'
9 import pluginsApis from './api.plugins'
10 import monitorApis from './api.monitor'
11 import { getConnections } from './connections'
12 -import { debounceAsync, isLocalHost, onOff } from './misc'
12 +import { debounceAsync, isLocalHost, onOff, wait } from './misc'
13 import _ from 'lodash'
14 import events from './events'
15 import { getFromAccount } from './perm'
@@ -52,8 +52,8 @@ export const adminApis: ApiHandlers = {
52 version: VERSION,
53 apiVersion: API_VERSION,
54 compatibleApiVersion: COMPATIBLE_API_VERSION,
55 - http: serverStatus(st.httpSrv, portCfg.get()),
56 - https: serverStatus(st.httpsSrv, httpsPortCfg.get()),
55 + http: await serverStatus(st.httpSrv, portCfg.get()),
56 + https: await serverStatus(st.httpsSrv, httpsPortCfg.get()),
57 urls: getUrls(),
58 proxyDetected: getProxyDetected(),
59 frpDetected: localhostAdmin.get() && !getProxyDetected()
@@ -61,9 +61,12 @@ export const adminApis: ApiHandlers = {
61 && await frpDebounced(),
62 }
63
64 - function serverStatus(h: typeof st.httpSrv, configuredPort?: number) {
64 + async function serverStatus(h: typeof st.httpSrv, configuredPort?: number) {
65 + const busy = await h.busy
66 + await wait(0) // simple trick to wait for also .error to be updated. If this trickery becomes necessary elsewhere, then we should make also error a Promise.
67 return {
66 - ..._.pick(h, ['listening', 'busy', 'error']),
68 + ..._.pick(h, ['listening', 'error']),
69 + busy,
70 port: (h?.address() as any)?.port || configuredPort,
71 }
72 }
server/src/listen.ts
+5 -4
@@ -12,7 +12,7 @@ import { debounceAsync, onlyTruthy, wait } from './misc'
12 import { ADMIN_URI, DEV } from './const'
13 import findProcess from 'find-process'
14
15 -interface ServerExtra { name: string, error?: string, busy?: string }
15 +interface ServerExtra { name: string, error?: string, busy?: Promise<string> }
16 let httpSrv: http.Server & ServerExtra
17 let httpsSrv: http.Server & ServerExtra
18
@@ -128,11 +128,12 @@ function startServer(srv: typeof httpSrv, { port, host }: StartServer) {
128 resolve(ad.port)
129 }).on('error', async e => {
130 srv.error = String(e)
131 + srv.busy = undefined
132 const { code } = e as any
133 if (code === 'EADDRINUSE') {
133 - const res = await findProcess('port', port)
134 - srv.busy = res[0]?.name
135 - srv.error = `couldn't listen on port ${port} used by ${srv.busy}`
134 + srv.busy = findProcess('port', port).then(res => res?.[0]?.name || '', () => '')
135 + console.debug("PROMISE")
136 + srv.error = `port ${port} busy: ${await srv.busy || "unknown process"}`
137 }
138 console.error(srv.name, srv.error)
139 const k = (srv === httpSrv? portCfg : httpsPortCfg).key()