Skip to content

Commit

Permalink
fix(app-shell): forward usb request errors and log (#15959)
Browse files Browse the repository at this point in the history
We were swallowing all errors from usb requests, which doesn't seem like
a great idea - it definitely led to lots of errors in browser logs.

Fixing this by propagating the errors back across the IPC allows the
browser-side tanstack query stuff to actually notice that there were
errors. Previously, those queries would just come back with `undefined`,
and stuff higher up the stack would see that its maintenance run data
was `undefined`, which is not something we test for or care about.

This seems to be a magic bullet that fixes various USB stability issues;
I'm a bit suspicious of it solving _everything_, but in my testing it
solves some things and it certainly can't hurt.

Closes RQA-2931 RQA-2933
  • Loading branch information
sfoster1 authored Aug 21, 2024
1 parent 8b081d7 commit 649b9aa
Show file tree
Hide file tree
Showing 3 changed files with 34 additions and 9 deletions.
13 changes: 9 additions & 4 deletions app-shell/src/system-info/usb-devices.ts
Original file line number Diff line number Diff line change
Expand Up @@ -133,10 +133,15 @@ function upstreamDeviceFromUsbDeviceWinAPI(
const parsePoshJsonOutputToWmiObjectArray = (
dump: string
): WmiObject[] => {
if (dump[0] === '[') {
return JSON.parse(dump) as WmiObject[]
} else {
return [JSON.parse(dump) as WmiObject]
try {
if (dump[0] === '[') {
return JSON.parse(dump) as WmiObject[]
} else {
return [JSON.parse(dump) as WmiObject]
}
} catch (e: any) {
log.error(`Failed to parse posh json output: ${dump}`)
throw e
}
}
if (dump.stderr !== '') {
Expand Down
24 changes: 20 additions & 4 deletions app-shell/src/usb.ts
Original file line number Diff line number Diff line change
Expand Up @@ -94,6 +94,19 @@ function reconstructFormData(ipcSafeFormData: IPCSafeFormData): FormData {
return result
}

const cloneError = (e: any): Record<string, unknown> =>
// eslint-disable-next-line @typescript-eslint/no-unsafe-argument
Object.entries(axios.isAxiosError(e) ? e.toJSON() : e).reduce<
Record<string, unknown>
>((acc, [k, v]) => {
try {
acc[k] = structuredClone(v)
return acc
} catch (e) {
return acc
}
}, {})

async function usbListener(
_event: IpcMainInvokeEvent,
config: AxiosRequestConfig
Expand All @@ -114,21 +127,24 @@ async function usbListener(

const usbHttpAgent = getSerialPortHttpAgent()
try {
usbLog.silly(`${config.method} ${config.url} timeout=${config.timeout}`)
const response = await axios.request({
httpAgent: usbHttpAgent,
...config,
data,
headers: { ...config.headers, ...formHeaders },
})
usbLog.silly(`${config.method} ${config.url} resolved ok`)
return {
error: false,
error: null,
data: response.data,
status: response.status,
statusText: response.statusText,
}
} catch (e) {
if (e instanceof Error) {
console.log(`axios request error ${e?.message ?? 'unknown'}`)
} catch (e: any) {
usbLog.info(`${config.method} ${config.url} failed: ${e}`)
return {
error: cloneError(e),
}
}
}
Expand Down
6 changes: 5 additions & 1 deletion app/src/redux/shell/remote.ts
Original file line number Diff line number Diff line change
Expand Up @@ -57,7 +57,11 @@ export async function appShellRequestor<Data>(
: data
const configProxy = { ...config, data: formDataProxy }

return await remote.ipcRenderer.invoke('usb:request', configProxy)
const result = await remote.ipcRenderer.invoke('usb:request', configProxy)
if (result?.error != null) {
throw result.error
}
return result
}

interface CallbackStore {
Expand Down

0 comments on commit 649b9aa

Please sign in to comment.