From ad81d35c712d523c286b4a859e93f45711a7bb95 Mon Sep 17 00:00:00 2001 From: Gregory Schier Date: Fri, 23 Aug 2024 13:20:48 -0700 Subject: [PATCH] Fix recursive plugin call locking --- .../src/gen/CallTemplateFunctionArgs.ts | 4 +- .../src/gen/RenderHttpRequestRequest.ts | 3 +- ...ateFunctionPurpose.ts => RenderPurpose.ts} | 2 +- plugin-runtime-types/src/index.ts | 2 +- plugin-runtime/src/index.worker.ts | 19 ++++---- src-tauri/src/lib.rs | 45 +++++++++---------- src-tauri/src/template_callback.rs | 8 ++-- src-tauri/yaak_plugin_runtime/src/events.rs | 9 ++-- src-tauri/yaak_plugin_runtime/src/manager.rs | 4 +- src-tauri/yaak_plugin_runtime/src/server.rs | 14 +++--- 10 files changed, 57 insertions(+), 53 deletions(-) rename plugin-runtime-types/src/gen/{CallTemplateFunctionPurpose.ts => RenderPurpose.ts} (63%) diff --git a/plugin-runtime-types/src/gen/CallTemplateFunctionArgs.ts b/plugin-runtime-types/src/gen/CallTemplateFunctionArgs.ts index fd7597f1..a4912f33 100644 --- a/plugin-runtime-types/src/gen/CallTemplateFunctionArgs.ts +++ b/plugin-runtime-types/src/gen/CallTemplateFunctionArgs.ts @@ -1,4 +1,4 @@ // This file was generated by [ts-rs](https://github.com/Aleph-Alpha/ts-rs). Do not edit this file manually. -import type { CallTemplateFunctionPurpose } from "./CallTemplateFunctionPurpose"; +import type { RenderPurpose } from "./RenderPurpose"; -export type CallTemplateFunctionArgs = { purpose: CallTemplateFunctionPurpose, values: { [key: string]: string }, }; +export type CallTemplateFunctionArgs = { purpose: RenderPurpose, values: { [key: string]: string }, }; diff --git a/plugin-runtime-types/src/gen/RenderHttpRequestRequest.ts b/plugin-runtime-types/src/gen/RenderHttpRequestRequest.ts index cd70dc07..6fa4c7f3 100644 --- a/plugin-runtime-types/src/gen/RenderHttpRequestRequest.ts +++ b/plugin-runtime-types/src/gen/RenderHttpRequestRequest.ts @@ -1,4 +1,5 @@ // This file was generated by [ts-rs](https://github.com/Aleph-Alpha/ts-rs). Do not edit this file manually. import type { HttpRequest } from "./HttpRequest"; +import type { RenderPurpose } from "./RenderPurpose"; -export type RenderHttpRequestRequest = { httpRequest: HttpRequest, }; +export type RenderHttpRequestRequest = { httpRequest: HttpRequest, purpose: RenderPurpose, }; diff --git a/plugin-runtime-types/src/gen/CallTemplateFunctionPurpose.ts b/plugin-runtime-types/src/gen/RenderPurpose.ts similarity index 63% rename from plugin-runtime-types/src/gen/CallTemplateFunctionPurpose.ts rename to plugin-runtime-types/src/gen/RenderPurpose.ts index 0b5e7689..ed6037ec 100644 --- a/plugin-runtime-types/src/gen/CallTemplateFunctionPurpose.ts +++ b/plugin-runtime-types/src/gen/RenderPurpose.ts @@ -1,3 +1,3 @@ // This file was generated by [ts-rs](https://github.com/Aleph-Alpha/ts-rs). Do not edit this file manually. -export type CallTemplateFunctionPurpose = "send" | "preview"; +export type RenderPurpose = "send" | "preview"; diff --git a/plugin-runtime-types/src/index.ts b/plugin-runtime-types/src/index.ts index e627b5c2..ad2cb96f 100644 --- a/plugin-runtime-types/src/index.ts +++ b/plugin-runtime-types/src/index.ts @@ -5,7 +5,6 @@ export type * from './themes'; export * from './gen/BootRequest'; export * from './gen/BootResponse'; export * from './gen/CallHttpRequestActionArgs'; -export * from './gen/CallTemplateFunctionPurpose'; export * from './gen/CallHttpRequestActionRequest'; export * from './gen/CallTemplateFunctionRequest'; export * from './gen/CallTemplateFunctionResponse'; @@ -48,6 +47,7 @@ export * from './gen/KeyValue'; export * from './gen/Model'; export * from './gen/RenderHttpRequestRequest'; export * from './gen/RenderHttpRequestResponse'; +export * from './gen/RenderPurpose'; export * from './gen/SendHttpRequestRequest'; export * from './gen/SendHttpRequestResponse'; export * from './gen/SendHttpRequestResponse'; diff --git a/plugin-runtime/src/index.worker.ts b/plugin-runtime/src/index.worker.ts index 4e1fb3e2..2fb9fe7d 100644 --- a/plugin-runtime/src/index.worker.ts +++ b/plugin-runtime/src/index.worker.ts @@ -64,6 +64,9 @@ new Promise(async (resolve, reject) => { } function sendEvent(event: InternalEvent) { + if (event.payload.type !== 'empty_response') { + console.log('Sending event to app', event.id, event.payload.type); + } parentPort!.postMessage(event); } @@ -77,8 +80,8 @@ new Promise(async (resolve, reject) => { const promise = new Promise(async (resolve) => { const cb = (event: InternalEvent) => { if (event.replyId === eventToSend.id) { - resolve(event.payload); // Not type-safe but oh well parentPort!.off('message', cb); // Unlisten, now that we're done + resolve(event.payload); // Not type-safe but oh well } }; parentPort!.on('message', cb); @@ -110,18 +113,18 @@ new Promise(async (resolve, reject) => { }, }, httpRequest: { - async getById({ id }) { - const payload = { type: 'get_http_request_by_id_request', id } as const; + async getById(args) { + const payload = { type: 'get_http_request_by_id_request', ...args } as const; const { httpRequest } = await sendAndWaitForReply(payload); return httpRequest; }, - async send({ httpRequest }) { - const payload = { type: 'send_http_request_request', httpRequest } as const; + async send(args) { + const payload = { type: 'send_http_request_request', ...args } as const; const { httpResponse } = await sendAndWaitForReply(payload); return httpResponse; }, - async render({ httpRequest }) { - const payload = { type: 'render_http_request_request', httpRequest } as const; + async render(args) { + const payload = { type: 'render_http_request_request', ...args } as const; const result = await sendAndWaitForReply(payload); return result.httpRequest; }, @@ -130,8 +133,6 @@ new Promise(async (resolve, reject) => { // Message comes into the plugin to be processed parentPort!.on('message', async ({ payload, id: replyId }: InternalEvent) => { - console.log(`Received ${payload.type}`); - try { if (payload.type === 'boot_request') { const payload: InternalEventPayload = { diff --git a/src-tauri/src/lib.rs b/src-tauri/src/lib.rs index 269fd950..a294ea67 100644 --- a/src-tauri/src/lib.rs +++ b/src-tauri/src/lib.rs @@ -1942,24 +1942,21 @@ fn monitor_plugin_events(app_handle: &AppHandle) { let plugin_manager: State<'_, PluginManager> = app_handle.state(); let (_rx_id, mut rx) = plugin_manager.subscribe().await; - let app_handle = app_handle.clone(); while let Some(event) = rx.recv().await { - let payload = match handle_plugin_event(&app_handle, &event).await { - Some(e) => e, - None => continue, - }; - if let Err(e) = plugin_manager.reply(&event, &payload).await { - warn!("Failed to reply to plugin manager: {}", e) - } + let app_handle = app_handle.clone(); + + // We might have recursive back-and-forth calls between app and plugin, so we don't + // want to block here + tauri::async_runtime::spawn(async move { + handle_plugin_event(&app_handle, &event).await; + }); } }); } -async fn handle_plugin_event( - app_handle: &AppHandle, - event: &InternalEvent, -) -> Option { - let event = match event.clone().payload { +async fn handle_plugin_event(app_handle: &AppHandle, event: &InternalEvent) { + info!("Got event to app {}", event.id); + let response_event: Option = match event.clone().payload { InternalEventPayload::CopyTextRequest(req) => { app_handle .clipboard() @@ -1992,7 +1989,7 @@ async fn handle_plugin_event( )) } InternalEventPayload::RenderHttpRequestRequest(req) => { - let w = get_focused_window_no_lock(app_handle)?; + let w = get_focused_window_no_lock(app_handle).expect("No focused window"); let workspace = get_workspace(app_handle, req.http_request.workspace_id.as_str()) .await .expect("Failed to get workspace for request"); @@ -2007,13 +2004,8 @@ async fn handle_plugin_event( Some(id) => get_environment(&w, id.as_str()).await.ok(), }; let cb = &*app_handle.state::(); - let rendered_http_request = render_http_request( - &req.http_request, - &workspace, - environment.as_ref(), - cb, - ) - .await; + let rendered_http_request = + render_http_request(&req.http_request, &workspace, environment.as_ref(), cb).await; Some(InternalEventPayload::RenderHttpRequestResponse( RenderHttpRequestResponse { http_request: rendered_http_request, @@ -2021,7 +2013,7 @@ async fn handle_plugin_event( )) } InternalEventPayload::SendHttpRequestRequest(req) => { - let w = get_focused_window_no_lock(app_handle)?; + let w = get_focused_window_no_lock(app_handle).expect("No focused window"); let url = w.url().unwrap(); let mut query_pairs = url.query_pairs(); @@ -2057,7 +2049,7 @@ async fn handle_plugin_event( let http_response = match result { Ok(r) => r, - Err(_e) => return None, + Err(_e) => return, }; Some(InternalEventPayload::SendHttpRequestResponse( @@ -2067,7 +2059,12 @@ async fn handle_plugin_event( _ => None, }; - event + if let Some(e) = response_event { + let plugin_manager: State<'_, PluginManager> = app_handle.state(); + if let Err(e) = plugin_manager.reply(&event, &e).await { + warn!("Failed to reply to plugin manager: {}", e) + } + } } // app_handle.get_focused_window locks, so this one is a non-locking version, safe for use in async context diff --git a/src-tauri/src/template_callback.rs b/src-tauri/src/template_callback.rs index 48bdcfec..0725a304 100644 --- a/src-tauri/src/template_callback.rs +++ b/src-tauri/src/template_callback.rs @@ -1,23 +1,23 @@ use std::collections::HashMap; use tauri::{AppHandle, Manager}; -use yaak_plugin_runtime::events::CallTemplateFunctionPurpose; +use yaak_plugin_runtime::events::RenderPurpose; use yaak_plugin_runtime::manager::PluginManager; use yaak_templates::TemplateCallback; #[derive(Clone)] pub struct PluginTemplateCallback { app_handle: AppHandle, - purpose: CallTemplateFunctionPurpose, + purpose: RenderPurpose, } impl PluginTemplateCallback { pub fn new(app_handle: AppHandle) -> PluginTemplateCallback { - PluginTemplateCallback { app_handle, purpose: CallTemplateFunctionPurpose::Preview } + PluginTemplateCallback { app_handle, purpose: RenderPurpose::Preview } } pub fn for_send(&self) -> PluginTemplateCallback { let mut v = self.clone(); - v.purpose = CallTemplateFunctionPurpose::Send; + v.purpose = RenderPurpose::Send; v } } diff --git a/src-tauri/yaak_plugin_runtime/src/events.rs b/src-tauri/yaak_plugin_runtime/src/events.rs index b0892044..fb82fd3a 100644 --- a/src-tauri/yaak_plugin_runtime/src/events.rs +++ b/src-tauri/yaak_plugin_runtime/src/events.rs @@ -153,6 +153,7 @@ pub struct CopyTextRequest { #[ts(export)] pub struct RenderHttpRequestRequest { pub http_request: HttpRequest, + pub purpose: RenderPurpose, } #[derive(Debug, Clone, Default, Serialize, Deserialize, TS)] @@ -289,21 +290,21 @@ pub struct CallTemplateFunctionResponse { #[serde(default, rename_all = "camelCase")] #[ts(export)] pub struct CallTemplateFunctionArgs { - pub purpose: CallTemplateFunctionPurpose, + pub purpose: RenderPurpose, pub values: HashMap, } #[derive(Debug, Clone, Serialize, Deserialize, TS)] #[serde(rename_all = "snake_case")] #[ts(export)] -pub enum CallTemplateFunctionPurpose { +pub enum RenderPurpose { Send, Preview, } -impl Default for CallTemplateFunctionPurpose { +impl Default for RenderPurpose { fn default() -> Self { - CallTemplateFunctionPurpose::Preview + RenderPurpose::Preview } } diff --git a/src-tauri/yaak_plugin_runtime/src/manager.rs b/src-tauri/yaak_plugin_runtime/src/manager.rs index 32ed4ec0..7d7cff11 100644 --- a/src-tauri/yaak_plugin_runtime/src/manager.rs +++ b/src-tauri/yaak_plugin_runtime/src/manager.rs @@ -1,6 +1,6 @@ use crate::error::Result; use crate::events::{ - CallHttpRequestActionRequest, CallTemplateFunctionArgs, CallTemplateFunctionPurpose, + CallHttpRequestActionRequest, CallTemplateFunctionArgs, RenderPurpose, CallTemplateFunctionRequest, CallTemplateFunctionResponse, FilterRequest, FilterResponse, GetHttpRequestActionsRequest, GetHttpRequestActionsResponse, GetTemplateFunctionsResponse, ImportRequest, ImportResponse, InternalEvent, InternalEventPayload, @@ -115,7 +115,7 @@ impl PluginManager { &self, fn_name: &str, args: HashMap, - purpose: CallTemplateFunctionPurpose, + purpose: RenderPurpose, ) -> Result> { let req = CallTemplateFunctionRequest { name: fn_name.to_string(), diff --git a/src-tauri/yaak_plugin_runtime/src/server.rs b/src-tauri/yaak_plugin_runtime/src/server.rs index b2f09557..58e89d84 100644 --- a/src-tauri/yaak_plugin_runtime/src/server.rs +++ b/src-tauri/yaak_plugin_runtime/src/server.rs @@ -53,7 +53,11 @@ impl PluginHandle { } pub async fn send(&self, event: &InternalEvent) -> Result<()> { - info!("Sending event {} {:?}", event.id, self.name().await); + info!( + "Sending event to plugin {} {:?}", + event.id, + self.name().await + ); self.to_plugin_tx .lock() .await @@ -90,9 +94,9 @@ impl PluginRuntimeGrpcServer { pub async fn subscribe(&self) -> (String, Receiver) { let (tx, rx) = mpsc::channel(128); - let id = generate_id(); - self.subscribers.lock().await.insert(id.clone(), tx); - (id, rx) + let rx_id = generate_id(); + self.subscribers.lock().await.insert(rx_id.clone(), tx); + (rx_id, rx) } pub async fn unsubscribe(&self, rx_id: &str) { @@ -394,7 +398,7 @@ impl PluginRuntime for PluginRuntimeGrpcServer { for tx in subscribers.values() { // Emit event to the channel for server to handle if let Err(e) = tx.try_send(event.clone()) { - println!("Failed to send to server channel. Receiver probably isn't listening: {:?}", e); + println!("Failed to send to server channel (n={}). Receiver probably isn't listening: {:?}", subscribers.len(), e); } }