diff --git a/CHANGELOG.md b/CHANGELOG.md index bf293e3e59..017fac3cff 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -6,6 +6,8 @@ #### Enso Compiler +- [Improvements to visualization handling][1804]. These improvements are fixing + possible performance issues around attaching and detaching visualizations. - [GeoMap visualization will ignore points with `null` coordinates][1775]. Now the presence of such points in the dataset will not break initial map positioning. @@ -15,6 +17,7 @@ [1775]: https://github.com/enso-org/ide/pull/1775 [1798]: https://github.com/enso-org/ide/pull/1798 +[1804]: https://github.com/enso-org/ide/pull/1804 # Enso 2.0.0-alpha.11 (2021-08-09) diff --git a/src/rust/ensogl/lib/core/src/display/render/pass.rs b/src/rust/ensogl/lib/core/src/display/render/pass.rs index c694d8b882..675d6f9543 100644 --- a/src/rust/ensogl/lib/core/src/display/render/pass.rs +++ b/src/rust/ensogl/lib/core/src/display/render/pass.rs @@ -90,7 +90,7 @@ impl Instance { } context.draw_buffers(&draw_buffers); context.bind_framebuffer(target,None); - Framebuffer {native,context} + Framebuffer {context,native} } } diff --git a/src/rust/ensogl/lib/core/src/display/scene.rs b/src/rust/ensogl/lib/core/src/display/scene.rs index c6790fc3fa..6be1391458 100644 --- a/src/rust/ensogl/lib/core/src/display/scene.rs +++ b/src/rust/ensogl/lib/core/src/display/scene.rs @@ -674,8 +674,8 @@ impl HardcodedLayers { , &tooltip_text , &cursor ]); - Self {viz,below_main,main,port_selection,label,above_nodes,above_nodes_text,panel,panel_text - ,node_searcher,node_searcher_mask,tooltip,tooltip_text,cursor,root,mask} + Self {root,viz,below_main,main,port_selection,label,above_nodes,above_nodes_text,panel + ,panel_text,node_searcher,node_searcher_mask,tooltip,tooltip_text,cursor,mask} } } diff --git a/src/rust/ide/lib/enso-protocol/src/language_server.rs b/src/rust/ide/lib/enso-protocol/src/language_server.rs index 85f02100a9..8297c3b8e4 100644 --- a/src/rust/ide/lib/enso-protocol/src/language_server.rs +++ b/src/rust/ide/lib/enso-protocol/src/language_server.rs @@ -7,6 +7,7 @@ //! This file tries to follow the scheme of the protocol specification. pub mod connection; +pub mod constants; pub mod response; #[cfg(test)] mod tests; @@ -162,3 +163,57 @@ trait API { , tags : Option> ) -> response::Completion; }} + + + +// ============== +// === Errors === +// ============== + +/// Check if the given `Error` value corresponds to an RPC call timeout. +/// +/// Recognizes both client- and server-side timeouts. +pub fn is_timeout_error(error:&failure::Error) -> bool { + use json_rpc::messages; + use json_rpc::RpcError; + use json_rpc::RpcError::*; + const TIMEOUT:i64 = constants::ErrorCodes::Timeout as i64; + matches!(error.downcast_ref::() + , Some(TimeoutError{..}) + | Some(RemoteError(messages::Error{code:TIMEOUT,..}))) +} + + + +// ============= +// === Tests === +// ============= + +#[cfg(test)] +mod test { + use super::*; + + #[test] + fn recognize_timeout_errors() { + type RpcError = json_rpc::RpcError::; + + // Server-side errors. + let text = r#"{"code":11,"message":"Request timeout"}"#; + let msg = serde_json::from_str::(text).unwrap(); + let error = RpcError::RemoteError(msg).into(); + assert!(is_timeout_error(&error)); + + let text = r#"{"code":2007,"message":"Evaluation of the visualisation expression failed"}"#; + let msg = serde_json::from_str::(text).unwrap(); + let error = RpcError::RemoteError(msg).into(); + assert!(!is_timeout_error(&error)); + + + // Client-side errors. + let error = RpcError::TimeoutError {millis:500}.into(); + assert!(is_timeout_error(&error)); + + let error = RpcError::LostConnection.into(); + assert!(!is_timeout_error(&error)); + } +} diff --git a/src/rust/ide/lib/enso-protocol/src/language_server/constants.rs b/src/rust/ide/lib/enso-protocol/src/language_server/constants.rs new file mode 100644 index 0000000000..d3d2a6fb50 --- /dev/null +++ b/src/rust/ide/lib/enso-protocol/src/language_server/constants.rs @@ -0,0 +1,25 @@ +//! Constants defined for the Language Server JSON-RPC API. + +use crate::prelude::*; + +/// Recognized error codes used by the Language Server messages. +/// +/// They follow `org.enso.jsonrpc.Error` object defined in the `enso` repository. +#[derive(Clone,Copy,Debug)] +pub enum ErrorCodes { + /// Server failed to parse JSON message. + ParseError = -32700, + /// JSON message sent was not a valid Request object. + InvalidRequest = -32600, + /// Requested method does not exist or is unavailable. + MethodNotFound = -32601, + /// Invalid method parameters. + InvalidParams = -32602, + /// Service error. + ServiceError = 1, + /// The requested method is not implemented. + NotImplementedError = 10, + /// Request timeout. + /// Note that timeout can also happen on the client side, as part of the Handler's logic. + Timeout = 11, +} diff --git a/src/rust/ide/lib/enso-protocol/src/language_server/tests.rs b/src/rust/ide/lib/enso-protocol/src/language_server/tests.rs index c713246246..67d8fdd669 100644 --- a/src/rust/ide/lib/enso-protocol/src/language_server/tests.rs +++ b/src/rust/ide/lib/enso-protocol/src/language_server/tests.rs @@ -512,7 +512,7 @@ fn test_execution_context() { let old_version = Sha3_224::new(b"Hello world!"); let new_version = Sha3_224::new(b"Hello, world!"); let path = main.clone(); - let edit = FileEdit {path,edits,old_version,new_version:new_version}; + let edit = FileEdit {path,edits,old_version,new_version}; test_request( |client| client.apply_text_file_edit(&edit), "text/applyEdit", diff --git a/src/rust/ide/lib/json-rpc/src/error.rs b/src/rust/ide/lib/json-rpc/src/error.rs index d55b07b768..71c2eabd2d 100644 --- a/src/rust/ide/lib/json-rpc/src/error.rs +++ b/src/rust/ide/lib/json-rpc/src/error.rs @@ -34,6 +34,9 @@ pub enum RpcError { MismatchedResponseType, /// Response timeout. + /// + /// Note that this represents the client-side timeout. Server-side timeout will be treated as + /// an [`RemoteError`]. #[allow(missing_docs)] #[fail(display = "Response timed out after {} ms.", millis)] TimeoutError{millis:u128}, diff --git a/src/rust/ide/lib/json-rpc/src/lib.rs b/src/rust/ide/lib/json-rpc/src/lib.rs index 1bdcd6f904..eb1f81d8d6 100644 --- a/src/rust/ide/lib/json-rpc/src/lib.rs +++ b/src/rust/ide/lib/json-rpc/src/lib.rs @@ -24,10 +24,11 @@ pub use api::RemoteMethodCall; pub use api::Result; pub use enso_prelude as prelude; pub use ensogl_system_web as ensogl; -pub use transport::Transport; -pub use transport::TransportEvent; +pub use error::RpcError; pub use handler::Event; pub use handler::Handler; +pub use transport::Transport; +pub use transport::TransportEvent; #[cfg(test)] pub use utils::test::traits::*; diff --git a/src/rust/ide/src/constants.rs b/src/rust/ide/src/constants.rs index eabee3d52f..77ffd85c25 100644 --- a/src/rust/ide/src/constants.rs +++ b/src/rust/ide/src/constants.rs @@ -22,11 +22,12 @@ pub const PROJECTS_MAIN_MODULE:&str = "Main"; /// Visualization folder where IDE can look for user-defined visualizations per project. pub const VISUALIZATION_DIRECTORY:&str = "visualization"; -/// How many times IDE will try to attach initial visualization on loading project. -/// -/// This is necessary, because the request will be timing out for a while, until the stdlib -/// compilation is done. -pub const INITIAL_VISUALIZATION_ATTACH_ATTEMPTS:usize = 50; +/// How many times IDE will try attaching visualization when there is a timeout error. +/// +/// Timeout error suggests that there might be nothing wrong with the request, just that the backend +/// is currently too busy to reply or that there is some connectivity hiccup. Thus, it makes sense +/// to give it a few more tries. +pub const ATTACHING_TIMEOUT_RETRIES:usize = 50; /// A module with language-specific constants. pub mod keywords { diff --git a/src/rust/ide/src/ide/integration/project.rs b/src/rust/ide/src/ide/integration/project.rs index f8b48806e0..999b4d9c3f 100644 --- a/src/rust/ide/src/ide/integration/project.rs +++ b/src/rust/ide/src/ide/integration/project.rs @@ -1733,11 +1733,11 @@ impl Model { executor::global::spawn(task); Ok(id) } - - - /// Repeatedly try to attach visualization to the node (as defined by the map). - /// - /// Up to `attempts` will be made. + + /// Try attaching visualization to the node. + /// + /// In case of timeout failure, retries up to total `attempts` count will be made. + /// For other kind of errors no further attempts will be made. fn try_attaching_visualization_task (&self, node_id:graph_editor::NodeId, visualizations_map:VisualizationMap, attempts:usize) -> impl Future>> { @@ -1756,11 +1756,16 @@ impl Model { {node_id}."); return AttachingResult::Attached(stream); } - Err(e) => { - error!(logger, "Failed to attach visualization {id} for node {node_id} \ - (attempt {i}): {e}"); + Err(e) if enso_protocol::language_server::is_timeout_error(&e) => { + warning!(logger, "Failed to attach visualization {id} for node \ + {node_id} (attempt {i}). Will retry, as it is a timeout error."); last_error = Some(e); } + Err(e) => { + warning!(logger, "Failed to attach visualization {id} for node \ + {node_id}: {e}"); + return AttachingResult::Failed(e) + } } } else { // If visualization is not present in the map, it means that UI detached it @@ -1789,7 +1794,7 @@ impl Model { ) -> impl Future { let graph_frp = self.view.graph().frp.clone_ref(); let map = visualizations_map.clone(); - let attempts = crate::constants::INITIAL_VISUALIZATION_ATTACH_ATTEMPTS; + let attempts = crate::constants::ATTACHING_TIMEOUT_RETRIES; let stream_fut = self.try_attaching_visualization_task(node_id,map,attempts); async move { // No need to log anything here, as `try_attaching_visualization_task` does this. diff --git a/src/rust/ide/view/graph-editor/src/lib.rs b/src/rust/ide/view/graph-editor/src/lib.rs index 724bbb0ea5..70e618c8ee 100644 --- a/src/rust/ide/view/graph-editor/src/lib.rs +++ b/src/rust/ide/view/graph-editor/src/lib.rs @@ -1270,8 +1270,7 @@ impl GraphEditorModelWithNetwork { // new one has been enabled. // TODO: Create a better API for updating the controller about visualisation changes // (see #896) - visualization_hidden_changed <- visualization_hidden.on_change(); - output.source.visualization_hidden <+ visualization_hidden_changed.constant(node_id); + output.source.visualization_hidden <+ visualization_hidden.constant(node_id); output.source.visualization_shown <+ visualization_shown.map2(&metadata,move |_,metadata| (node_id,metadata.clone()));