-
Notifications
You must be signed in to change notification settings - Fork 178
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
fix(app-shell): forward usb request errors and log #15959
fix(app-shell): forward usb request errors and log #15959
Conversation
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. While we're at it, let's get some more in-detph logging of usb requests.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Exciting!
@@ -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< |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Just curious, in practice, is this ever not an axios error? It's probably safer to do this regardless.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
not as far as I know, but this way if we ever make a new bug it will at least get reported in the browser logs
return await remote.ipcRenderer.invoke('usb:request', configProxy) | ||
const result = await remote.ipcRenderer.invoke('usb:request', configProxy) | ||
if (result?.error != null) { | ||
throw result.error |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ah
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 wasundefined
, 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