From b0a469faaf0fd23e3a3ebab85f40415a3e9096ae Mon Sep 17 00:00:00 2001 From: Georgy Moshkin <gmoshkin@picodata.io> Date: Tue, 9 Jul 2024 15:43:00 +0300 Subject: [PATCH] fix: report NoSuchInstance error in RPC client --- picoplugin/src/error_code.rs | 2 ++ src/plugin/rpc/client.rs | 8 ++++++-- src/traft/error.rs | 3 +++ test/conftest.py | 1 + test/int/test_plugin.py | 13 ++++++++----- 5 files changed, 20 insertions(+), 7 deletions(-) diff --git a/picoplugin/src/error_code.rs b/picoplugin/src/error_code.rs index 21801786d9..6c23eb3399 100644 --- a/picoplugin/src/error_code.rs +++ b/picoplugin/src/error_code.rs @@ -35,6 +35,8 @@ tarantool::define_enum_with_introspection! { ServiceNotAvailable = 10014, WrongPluginVersion = 10015, + NoSuchInstance = 10016, + // TODO: put in particular compare-and-swap related ones, but also other ones /// Not an actual error code, just designates the start of the range. diff --git a/src/plugin/rpc/client.rs b/src/plugin/rpc/client.rs index 5384a53c82..68100c6221 100644 --- a/src/plugin/rpc/client.rs +++ b/src/plugin/rpc/client.rs @@ -333,9 +333,13 @@ fn check_route_to_instance( plugin_version: &ident.version, service_name: service, })?; + #[rustfmt::skip] let Some(tuple) = res else { - #[rustfmt::skip] - return Err(BoxError::new(ErrorCode::ServiceNotStarted, format!("service '{ident}.{service}' is not running on {instance_id}")).into()); + if node.storage.instances.get_raw(instance_id).is_ok() { + return Err(BoxError::new(ErrorCode::ServiceNotStarted, format!("service '{ident}.{service}' is not running on {instance_id}")).into()); + } else { + return Err(BoxError::new(ErrorCode::NoSuchInstance, format!("instance with instance_id \"{instance_id}\" not found")).into()); + } }; let res = tuple .field(ServiceRouteItem::FIELD_POISON) diff --git a/src/traft/error.rs b/src/traft/error.rs index 00c472b995..8cbb7f6f6f 100644 --- a/src/traft/error.rs +++ b/src/traft/error.rs @@ -204,6 +204,9 @@ impl IntoBoxError for Error { Self::Tarantool(e) => e.into_box_error(), Self::NotALeader => BoxError::new(ErrorCode::NotALeader, "not a leader"), Self::Other(e) => BoxError::new(ErrorCode::Other, e.to_string()), + Self::NoSuchInstance { .. } => { + BoxError::new(ErrorCode::NoSuchInstance, self.to_string()) + } // TODO: give other error types specific codes other => BoxError::new(ErrorCode::Other, other.to_string()), } diff --git a/test/conftest.py b/test/conftest.py index e4d4d1c368..855a4193ba 100644 --- a/test/conftest.py +++ b/test/conftest.py @@ -58,6 +58,7 @@ PICO_SERVICE_ID = 32 # Note: our tarantool.error.tnt_strerror only knows about first 113 error codes.. class ErrorCode: Loading = 116 + NoSuchInstance = 10016 def eprint(*args, **kwargs): diff --git a/test/int/test_plugin.py b/test/int/test_plugin.py index ee136930cb..e6a32f5c39 100644 --- a/test/int/test_plugin.py +++ b/test/int/test_plugin.py @@ -13,6 +13,9 @@ from conftest import ( log_crawler, ) from decimal import Decimal +from conftest import ( + ErrorCode, +) _3_SEC = 3 _DEFAULT_CFG = {"foo": True, "bar": 101, "baz": ["one", "two", "three"]} @@ -1793,11 +1796,7 @@ def test_plugin_rpc_sdk_send_request(cluster: Cluster): i1.call(".proc_rpc_dispatch", "/proxy", msgpack.dumps(input), context) # Check requesting RPC to unknown instance - with pytest.raises( - TarantoolError, - # FIXME: do a better error message - match=f"service '{plugin_name}:0.1.0.{service_name}' is not running on NO_SUCH_INSTANCE", - ): + with pytest.raises(TarantoolError) as e: context = make_context() input = dict( path="/ping", @@ -1805,6 +1804,10 @@ def test_plugin_rpc_sdk_send_request(cluster: Cluster): input=msgpack.dumps([]), ) i1.call(".proc_rpc_dispatch", "/proxy", msgpack.dumps(input), context) + assert e.value.args[:2] == ( + ErrorCode.NoSuchInstance, + 'instance with instance_id "NO_SUCH_INSTANCE" not found', + ) # Check requesting RPC to unknown replicaset with pytest.raises( -- GitLab