refactor: use Map registry for bridge callbacks with unique IDs and timeout
- commit
- 5c01908df7b1c28b0c5c79d177f2300896c52c93
- author
- brice · · 2026-01-24 16:38 UTC
- parent
- 200ec03
- signature
- GPG Verified
CD88F9E564E0102646FB6077A1A07C7C7C3E349D· arch-gpg Verified for the committer email
1 file · +46 -11
M webui/src/lib/bridge.ts +46 -11
| @@ -76,13 +76,27 @@ export function postMessage(msg: BridgeMessage): void { | |||
| 76 | 76 | bridge.postMessage(plainMsg); | |
| 77 | 77 | } | |
| 78 | 78 | ||
| 79 | + | /** | |
| 80 | + | * Internal callback registry using Map for tracking pending requests. | |
| 81 | + | * Callbacks must also be exposed on window for Go backend to call, | |
| 82 | + | * but we track them here for proper cleanup and to prevent leaks. | |
| 83 | + | */ | |
| 84 | + | const pendingCallbacks = new Map<string, { | |
| 85 | + | cleanup: () => void; | |
| 86 | + | timeoutId: ReturnType<typeof setTimeout>; | |
| 87 | + | }>(); | |
| 88 | + | ||
| 89 | + | let callbackCounter = 0; | |
| 90 | + | const CALLBACK_TIMEOUT_MS = 30000; | |
| 91 | + | ||
| 79 | 92 | /** | |
| 80 | 93 | * Post a message and set up success/error callbacks with automatic cleanup. | |
| 94 | + | * Uses unique IDs to prevent naming conflicts between concurrent requests. | |
| 81 | 95 | * | |
| 82 | 96 | * @param type - Message type (e.g., "get_keybindings", "set_keybinding") | |
| 83 | 97 | * @param payload - Message payload (will be serialized to strip Proxies) | |
| 84 | - | * @param successCallback - Window function name to call on success | |
| 85 | - | * @param errorCallback - Window function name to call on error | |
| 98 | + | * @param successCallback - Base name for success callback (will be made unique) | |
| 99 | + | * @param errorCallback - Base name for error callback (will be made unique) | |
| 86 | 100 | * @param onSuccess - Handler for success response | |
| 87 | 101 | * @param onError - Handler for error response | |
| 88 | 102 | */ | |
| @@ -94,22 +108,40 @@ export function postMessageWithCallbacks<TResponse, TPayload extends Record<stri | |||
| 94 | 108 | onSuccess: (response: TResponse) => void, | |
| 95 | 109 | onError: (error: string) => void, | |
| 96 | 110 | ): void { | |
| 97 | - | // Cleanup function to remove callbacks from window | |
| 111 | + | const id = ++callbackCounter; | |
| 112 | + | const successKey = `${successCallback}_${id}`; | |
| 113 | + | const errorKey = `${errorCallback}_${id}`; | |
| 114 | + | ||
| 115 | + | // Cleanup removes from both window and our tracking Map | |
| 98 | 116 | const cleanup = () => { | |
| 117 | + | const entry = pendingCallbacks.get(successKey); | |
| 118 | + | if (entry) { | |
| 119 | + | clearTimeout(entry.timeoutId); | |
| 120 | + | pendingCallbacks.delete(successKey); | |
| 121 | + | } | |
| 99 | 122 | // eslint-disable-next-line @typescript-eslint/no-explicit-any | |
| 100 | - | delete (window as any)[successCallback]; | |
| 123 | + | delete (window as any)[successKey]; | |
| 101 | 124 | // eslint-disable-next-line @typescript-eslint/no-explicit-any | |
| 102 | - | delete (window as any)[errorCallback]; | |
| 125 | + | delete (window as any)[errorKey]; | |
| 103 | 126 | }; | |
| 104 | 127 | ||
| 105 | - | // Set up callbacks on window with automatic cleanup | |
| 128 | + | // Timeout prevents memory leaks if backend never responds | |
| 129 | + | const timeoutId = setTimeout(() => { | |
| 130 | + | cleanup(); | |
| 131 | + | onError("Request timed out"); | |
| 132 | + | }, CALLBACK_TIMEOUT_MS); | |
| 133 | + | ||
| 134 | + | // Track in our Map for proper lifecycle management | |
| 135 | + | pendingCallbacks.set(successKey, { cleanup, timeoutId }); | |
| 136 | + | ||
| 137 | + | // Expose on window for Go backend (it calls window[callbackName](response)) | |
| 106 | 138 | // eslint-disable-next-line @typescript-eslint/no-explicit-any | |
| 107 | - | (window as any)[successCallback] = (response: TResponse) => { | |
| 139 | + | (window as any)[successKey] = (response: TResponse) => { | |
| 108 | 140 | cleanup(); | |
| 109 | 141 | onSuccess(response); | |
| 110 | 142 | }; | |
| 111 | 143 | // eslint-disable-next-line @typescript-eslint/no-explicit-any | |
| 112 | - | (window as any)[errorCallback] = (msg: string) => { | |
| 144 | + | (window as any)[errorKey] = (msg: unknown) => { | |
| 113 | 145 | cleanup(); | |
| 114 | 146 | onError(typeof msg === "string" ? msg : "Unknown error"); | |
| 115 | 147 | }; | |
| @@ -118,11 +150,14 @@ export function postMessageWithCallbacks<TResponse, TPayload extends Record<stri | |||
| 118 | 150 | postMessage({ | |
| 119 | 151 | type, | |
| 120 | 152 | webviewId: getWebViewId(), | |
| 121 | - | payload, | |
| 153 | + | payload: { | |
| 154 | + | ...payload, | |
| 155 | + | successCallback: successKey, | |
| 156 | + | errorCallback: errorKey, | |
| 157 | + | }, | |
| 122 | 158 | }); | |
| 123 | 159 | } catch (err) { | |
| 124 | 160 | cleanup(); | |
| 125 | - | const message = err instanceof Error ? err.message : String(err); | |
| 126 | - | onError(message); | |
| 161 | + | onError(err instanceof Error ? err.message : String(err)); | |
| 127 | 162 | } | |
| 128 | 163 | } | |
| @@ -76,13 +76,27 @@ export function postMessage(msg: BridgeMessage): void { | |||||
| 76 | bridge.postMessage(plainMsg); | 76 | bridge.postMessage(plainMsg); | ||
| 77 | } | 77 | } | ||
| 78 | 78 | ||||
| 79 | + | /** | |||
| 80 | + | * Internal callback registry using Map for tracking pending requests. | |||
| 81 | + | * Callbacks must also be exposed on window for Go backend to call, | |||
| 82 | + | * but we track them here for proper cleanup and to prevent leaks. | |||
| 83 | + | */ | |||
| 84 | + | const pendingCallbacks = new Map<string, { | |||
| 85 | + | cleanup: () => void; | |||
| 86 | + | timeoutId: ReturnType<typeof setTimeout>; | |||
| 87 | + | }>(); | |||
| 88 | + | ||||
| 89 | + | let callbackCounter = 0; | |||
| 90 | + | const CALLBACK_TIMEOUT_MS = 30000; | |||
| 91 | + | ||||
| 79 | /** | 92 | /** | ||
| 80 | * Post a message and set up success/error callbacks with automatic cleanup. | 93 | * Post a message and set up success/error callbacks with automatic cleanup. | ||
| 94 | + | * Uses unique IDs to prevent naming conflicts between concurrent requests. | |||
| 81 | * | 95 | * | ||
| 82 | * @param type - Message type (e.g., "get_keybindings", "set_keybinding") | 96 | * @param type - Message type (e.g., "get_keybindings", "set_keybinding") | ||
| 83 | * @param payload - Message payload (will be serialized to strip Proxies) | 97 | * @param payload - Message payload (will be serialized to strip Proxies) | ||
| 84 | - | * @param successCallback - Window function name to call on success | 98 | + | * @param successCallback - Base name for success callback (will be made unique) |
| 85 | - | * @param errorCallback - Window function name to call on error | 99 | + | * @param errorCallback - Base name for error callback (will be made unique) |
| 86 | * @param onSuccess - Handler for success response | 100 | * @param onSuccess - Handler for success response | ||
| 87 | * @param onError - Handler for error response | 101 | * @param onError - Handler for error response | ||
| 88 | */ | 102 | */ | ||
| @@ -94,22 +108,40 @@ export function postMessageWithCallbacks<TResponse, TPayload extends Record<stri | |||||
| 94 | onSuccess: (response: TResponse) => void, | 108 | onSuccess: (response: TResponse) => void, | ||
| 95 | onError: (error: string) => void, | 109 | onError: (error: string) => void, | ||
| 96 | ): void { | 110 | ): void { | ||
| 97 | - | // Cleanup function to remove callbacks from window | 111 | + | const id = ++callbackCounter; |
| 112 | + | const successKey = `${successCallback}_${id}`; | |||
| 113 | + | const errorKey = `${errorCallback}_${id}`; | |||
| 114 | + | ||||
| 115 | + | // Cleanup removes from both window and our tracking Map | |||
| 98 | const cleanup = () => { | 116 | const cleanup = () => { | ||
| 117 | + | const entry = pendingCallbacks.get(successKey); | |||
| 118 | + | if (entry) { | |||
| 119 | + | clearTimeout(entry.timeoutId); | |||
| 120 | + | pendingCallbacks.delete(successKey); | |||
| 121 | + | } | |||
| 99 | // eslint-disable-next-line @typescript-eslint/no-explicit-any | 122 | // eslint-disable-next-line @typescript-eslint/no-explicit-any | ||
| 100 | - | delete (window as any)[successCallback]; | 123 | + | delete (window as any)[successKey]; |
| 101 | // eslint-disable-next-line @typescript-eslint/no-explicit-any | 124 | // eslint-disable-next-line @typescript-eslint/no-explicit-any | ||
| 102 | - | delete (window as any)[errorCallback]; | 125 | + | delete (window as any)[errorKey]; |
| 103 | }; | 126 | }; | ||
| 104 | 127 | ||||
| 105 | - | // Set up callbacks on window with automatic cleanup | 128 | + | // Timeout prevents memory leaks if backend never responds |
| 129 | + | const timeoutId = setTimeout(() => { | |||
| 130 | + | cleanup(); | |||
| 131 | + | onError("Request timed out"); | |||
| 132 | + | }, CALLBACK_TIMEOUT_MS); | |||
| 133 | + | ||||
| 134 | + | // Track in our Map for proper lifecycle management | |||
| 135 | + | pendingCallbacks.set(successKey, { cleanup, timeoutId }); | |||
| 136 | + | ||||
| 137 | + | // Expose on window for Go backend (it calls window[callbackName](response)) | |||
| 106 | // eslint-disable-next-line @typescript-eslint/no-explicit-any | 138 | // eslint-disable-next-line @typescript-eslint/no-explicit-any | ||
| 107 | - | (window as any)[successCallback] = (response: TResponse) => { | 139 | + | (window as any)[successKey] = (response: TResponse) => { |
| 108 | cleanup(); | 140 | cleanup(); | ||
| 109 | onSuccess(response); | 141 | onSuccess(response); | ||
| 110 | }; | 142 | }; | ||
| 111 | // eslint-disable-next-line @typescript-eslint/no-explicit-any | 143 | // eslint-disable-next-line @typescript-eslint/no-explicit-any | ||
| 112 | - | (window as any)[errorCallback] = (msg: string) => { | 144 | + | (window as any)[errorKey] = (msg: unknown) => { |
| 113 | cleanup(); | 145 | cleanup(); | ||
| 114 | onError(typeof msg === "string" ? msg : "Unknown error"); | 146 | onError(typeof msg === "string" ? msg : "Unknown error"); | ||
| 115 | }; | 147 | }; | ||
| @@ -118,11 +150,14 @@ export function postMessageWithCallbacks<TResponse, TPayload extends Record<stri | |||||
| 118 | postMessage({ | 150 | postMessage({ | ||
| 119 | type, | 151 | type, | ||
| 120 | webviewId: getWebViewId(), | 152 | webviewId: getWebViewId(), | ||
| 121 | - | payload, | 153 | + | payload: { |
| 154 | + | ...payload, | |||
| 155 | + | successCallback: successKey, | |||
| 156 | + | errorCallback: errorKey, | |||
| 157 | + | }, | |||
| 122 | }); | 158 | }); | ||
| 123 | } catch (err) { | 159 | } catch (err) { | ||
| 124 | cleanup(); | 160 | cleanup(); | ||
| 125 | - | const message = err instanceof Error ? err.message : String(err); | 161 | + | onError(err instanceof Error ? err.message : String(err)); |
| 126 | - | onError(message); | |||
| 127 | } | 162 | } | ||
| 128 | } | 163 | } | ||