From e179d23e91fd91566909a86723976024506c804c Mon Sep 17 00:00:00 2001 From: German <28149841+germa89@users.noreply.github.com> Date: Tue, 30 Jul 2024 17:08:44 +0200 Subject: [PATCH 01/12] feat: adding more descriptive message when the message limit has been exceeded. --- src/ansys/mapdl/core/errors.py | 43 +++++++++++++++++++++++++++++----- 1 file changed, 37 insertions(+), 6 deletions(-) diff --git a/src/ansys/mapdl/core/errors.py b/src/ansys/mapdl/core/errors.py index e3b8c536be..87efd3e3bd 100644 --- a/src/ansys/mapdl/core/errors.py +++ b/src/ansys/mapdl/core/errors.py @@ -270,6 +270,13 @@ def __init__(self, msg=""): super().__init__(msg) +class MapdlgRPCError(MapdlRuntimeError): + """Raised when gRPC issues are found""" + + def __init__(self, msg=""): + super().__init__(msg) + + # handler for protect_grpc def handler(sig, frame): # pragma: no cover """Pass signal to custom interrupt handler.""" @@ -316,7 +323,7 @@ def wrapper(*args, **kwargs): caller = func.__name__ if cmd: - msg_ = f"running:\n{cmd}\ncalled by:\n{caller}" + msg_ = f"running:\n\t{cmd}\ncalled by:\n\t{caller}\n" else: msg_ = f"calling:{caller}\nwith the following arguments:\nargs: {list(*args)}\nkwargs: {list(**kwargs_)}" @@ -325,11 +332,35 @@ def wrapper(*args, **kwargs): elif hasattr(args[0], "_mapdl"): mapdl = args[0]._mapdl - # Must close unfinished processes - mapdl._close_process() - raise MapdlExitedError( - f"MAPDL server connection terminated unexpectedly while {msg_}\nwith the following error\n{error}" - ) from None + if "Received message larger than max" in error.details(): + try: + lim_ = int(error.details().split("(")[1].split("vs")[0]) + except IndexError: + lim_ = int(512 * 1024**2) + + raise MapdlgRPCError( + f"{error.details()}. " + "You can try to increase the gRPC message length size using 'PYMAPDL_MAX_MESSAGE_LENGTH'" + " environment variable. For instance:\n\n" + f"$ export PYMAPDL_MAX_MESSAGE_LENGTH={lim_}" + ) + + else: + # Generic error + msg = ( + f"MAPDL server connection terminated unexpectedly while {msg_}\n" + "Error:\n" + f"\t{error.details()}\n" + f"Full error:\n{error}" + ) + # Test if MAPDL is alive or not. + if mapdl.is_alive: + raise MapdlRuntimeError(msg) + + else: + # Must close unfinished processes + mapdl._close_process() + raise MapdlExitedError(msg) if threading.current_thread().__class__.__name__ == "_MainThread": received_interrupt = bool(SIGINT_TRACKER) From 2ebc8dc52def3f4fc6a5def680aee64aa155454d Mon Sep 17 00:00:00 2001 From: German <28149841+germa89@users.noreply.github.com> Date: Tue, 30 Jul 2024 18:13:03 +0200 Subject: [PATCH 02/12] refactor: moving out the generic error raising.\nPreparing for custom gRPC error codes. --- src/ansys/mapdl/core/errors.py | 123 ++++++++++++++++++--------------- 1 file changed, 68 insertions(+), 55 deletions(-) diff --git a/src/ansys/mapdl/core/errors.py b/src/ansys/mapdl/core/errors.py index 87efd3e3bd..d5f5df9f68 100644 --- a/src/ansys/mapdl/core/errors.py +++ b/src/ansys/mapdl/core/errors.py @@ -21,13 +21,12 @@ # SOFTWARE. """PyMAPDL specific errors""" - from functools import wraps import signal import threading from typing import Callable, Optional -from grpc._channel import _InactiveRpcError, _MultiThreadedRendezvous +import grpc from ansys.mapdl.core import LOG as logger @@ -309,59 +308,26 @@ def wrapper(*args, **kwargs): # Capture gRPC exceptions try: out = func(*args, **kwargs) - except (_InactiveRpcError, _MultiThreadedRendezvous) as error: - # can't use isinstance here due to circular imports - try: - class_name = args[0].__class__.__name__ - except: - class_name = "" - - # trying to get "cmd" argument: - cmd = args[1] if len(args) >= 2 else "" - cmd = kwargs.get("cmd", cmd) - - caller = func.__name__ - - if cmd: - msg_ = f"running:\n\t{cmd}\ncalled by:\n\t{caller}\n" - else: - msg_ = f"calling:{caller}\nwith the following arguments:\nargs: {list(*args)}\nkwargs: {list(**kwargs_)}" - - if class_name == "MapdlGrpc": - mapdl = args[0] - elif hasattr(args[0], "_mapdl"): - mapdl = args[0]._mapdl - - if "Received message larger than max" in error.details(): - try: - lim_ = int(error.details().split("(")[1].split("vs")[0]) - except IndexError: - lim_ = int(512 * 1024**2) - - raise MapdlgRPCError( - f"{error.details()}. " - "You can try to increase the gRPC message length size using 'PYMAPDL_MAX_MESSAGE_LENGTH'" - " environment variable. For instance:\n\n" - f"$ export PYMAPDL_MAX_MESSAGE_LENGTH={lim_}" - ) - - else: - # Generic error - msg = ( - f"MAPDL server connection terminated unexpectedly while {msg_}\n" - "Error:\n" - f"\t{error.details()}\n" - f"Full error:\n{error}" - ) - # Test if MAPDL is alive or not. - if mapdl.is_alive: - raise MapdlRuntimeError(msg) - - else: - # Must close unfinished processes - mapdl._close_process() - raise MapdlExitedError(msg) - + except grpc.RpcError as error: + # Custom errors + if error.code() == grpc.StatusCode.RESOURCE_EXHAUSTED: + if "Received message larger than max" in error.details(): + try: + lim_ = int(error.details().split("(")[1].split("vs")[0]) + except IndexError: + lim_ = int(512 * 1024**2) + + raise MapdlgRPCError( + f"RESOURCE_EXHAUSTED: {error.details()}. " + "You can try to increase the gRPC message length size using 'PYMAPDL_MAX_MESSAGE_LENGTH'" + " environment variable. For instance:\n\n" + f"$ export PYMAPDL_MAX_MESSAGE_LENGTH={lim_}" + ) + + # Generic error + handle_generic_grpc_error(error, func, args, kwargs) + + # No exceptions if threading.current_thread().__class__.__name__ == "_MainThread": received_interrupt = bool(SIGINT_TRACKER) @@ -378,6 +344,53 @@ def wrapper(*args, **kwargs): return wrapper +def handle_generic_grpc_error(error, func, args, kwargs): + """Handle non-custom gRPC errors""" + + # can't use isinstance here due to circular imports + try: + class_name = args[0].__class__.__name__ + except: + class_name = "" + + # trying to get "cmd" argument: + cmd = args[1] if len(args) >= 2 else "" + cmd = kwargs.get("cmd", cmd) + + caller = func.__name__ + + if cmd: + msg_ = f"running:\n\t{cmd}\ncalled by:\n\t{caller}\n" + else: + msg_ = f"calling:{caller}\nwith the following arguments:\nargs: {list(*args)}\nkwargs: {list(**kwargs_)}" + + if class_name == "MapdlGrpc": + mapdl = args[0] + elif hasattr(args[0], "_mapdl"): + mapdl = args[0]._mapdl + + msg = ( + f"MAPDL server connection terminated unexpectedly while {msg_}\n" + "Error:\n" + f"\t{error.details()}\n" + f"Full error:\n{error}" + ) + + # MAPDL gRPC is unavailable. + if error.code() == grpc.StatusCode.UNAVAILABLE: + raise MapdlExitedError(msg) + + # Generic error + # Test if MAPDL is alive or not. + if mapdl.is_alive: + raise MapdlRuntimeError(msg) + + else: + # Must close unfinished processes + mapdl._close_process() + raise MapdlExitedError(msg) + + def protect_from( exception, match: Optional[str] = None, condition: Optional[bool] = None ) -> Callable: From 6e0677abd41d5734e104303f1cc8eafde3a4168e Mon Sep 17 00:00:00 2001 From: German <28149841+germa89@users.noreply.github.com> Date: Tue, 30 Jul 2024 18:13:21 +0200 Subject: [PATCH 03/12] fix: typo --- src/ansys/mapdl/core/parameters.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/ansys/mapdl/core/parameters.py b/src/ansys/mapdl/core/parameters.py index 9014eeb840..32c96bbef8 100644 --- a/src/ansys/mapdl/core/parameters.py +++ b/src/ansys/mapdl/core/parameters.py @@ -548,7 +548,7 @@ def _get_parameter_array(self, parm_name, shape): if not escaped: # pragma: no cover raise MapdlRuntimeError( f"The array '{parm_name}' has a number format " - "that could not be read using '{format_str}'." + f"that could not be read using '{format_str}'." ) arr_flat = np.fromstring(output, sep="\n").reshape(shape) From 2ccd7d5dc840c7361d64a83ee19285e38cccd396 Mon Sep 17 00:00:00 2001 From: German <28149841+germa89@users.noreply.github.com> Date: Tue, 30 Jul 2024 18:16:32 +0200 Subject: [PATCH 04/12] feat: adding message limit to database channel too --- src/ansys/mapdl/core/database/database.py | 10 +++++++--- 1 file changed, 7 insertions(+), 3 deletions(-) diff --git a/src/ansys/mapdl/core/database/database.py b/src/ansys/mapdl/core/database/database.py index dd05f09ce0..77258d07b6 100644 --- a/src/ansys/mapdl/core/database/database.py +++ b/src/ansys/mapdl/core/database/database.py @@ -33,8 +33,7 @@ import grpc from ansys.mapdl.core.errors import MapdlConnectionError - -from ..mapdl_grpc import MapdlGrpc +from ansys.mapdl.core.mapdl_grpc import MAX_MESSAGE_LENGTH, MapdlGrpc MINIMUM_MAPDL_VERSION = "21.1" FAILING_DATABASE_MAPDL = ["24.1", "24.2"] @@ -280,7 +279,12 @@ def start(self, timeout=10): self._server = {"ip": self._ip, "port": db_port} self._channel_str = f"{self._ip}:{db_port}" - self._channel = grpc.insecure_channel(self._channel_str) + self._channel = grpc.insecure_channel( + self._channel_str, + options=[ + ("grpc.max_receive_message_length", MAX_MESSAGE_LENGTH), + ], + ) self._state = grpc.channel_ready_future(self._channel) self._stub = mapdl_db_pb2_grpc.MapdlDbServiceStub(self._channel) From 59decdde0fea793a81a0b3632b526be3b3f8191f Mon Sep 17 00:00:00 2001 From: pyansys-ci-bot <92810346+pyansys-ci-bot@users.noreply.github.com> Date: Tue, 30 Jul 2024 16:22:53 +0000 Subject: [PATCH 05/12] chore: adding changelog file 3319.miscellaneous.md --- doc/changelog.d/3319.miscellaneous.md | 1 + 1 file changed, 1 insertion(+) create mode 100644 doc/changelog.d/3319.miscellaneous.md diff --git a/doc/changelog.d/3319.miscellaneous.md b/doc/changelog.d/3319.miscellaneous.md new file mode 100644 index 0000000000..2042f86d5c --- /dev/null +++ b/doc/changelog.d/3319.miscellaneous.md @@ -0,0 +1 @@ +feat: adding more descriptive errors \ No newline at end of file From 95a309d671ea99d8857ca64dbc03bf2d7f64a23f Mon Sep 17 00:00:00 2001 From: German <28149841+germa89@users.noreply.github.com> Date: Tue, 30 Jul 2024 18:27:41 +0200 Subject: [PATCH 06/12] test: adding tests --- tests/test_grpc.py | 69 ++++++++++++++++++++++++++++++++++++++++------ 1 file changed, 61 insertions(+), 8 deletions(-) diff --git a/tests/test_grpc.py b/tests/test_grpc.py index 3f5745b993..1687ee3a6f 100644 --- a/tests/test_grpc.py +++ b/tests/test_grpc.py @@ -26,6 +26,7 @@ import shutil import sys +import grpc import pytest from ansys.mapdl.core import examples @@ -33,8 +34,10 @@ from ansys.mapdl.core.errors import ( MapdlCommandIgnoredError, MapdlExitedError, + MapdlgRPCError, MapdlRuntimeError, ) +from ansys.mapdl.core.mapdl_grpc import MAX_MESSAGE_LENGTH, MapdlGrpc from ansys.mapdl.core.misc import random_string PATH = os.path.dirname(os.path.abspath(__file__)) @@ -100,20 +103,20 @@ def setup_for_cmatrix(mapdl, cleared): mapdl.run("/solu") -def test_connect_via_channel(mapdl): - """Validate MapdlGrpc can be created directly from a channel.""" - - import grpc - - from ansys.mapdl.core.mapdl_grpc import MAX_MESSAGE_LENGTH, MapdlGrpc - +@pytest.fixture(scope="function") +def grpc_channel(mapdl): channel = grpc.insecure_channel( mapdl._channel_str, options=[ ("grpc.max_receive_message_length", MAX_MESSAGE_LENGTH), ], ) - mapdl = MapdlGrpc(channel=channel) + return channel + + +def test_connect_via_channel(grpc_channel): + """Validate MapdlGrpc can be created directly from a channel.""" + mapdl = MapdlGrpc(channel=grpc_channel) assert mapdl.is_alive @@ -570,3 +573,53 @@ def test__check_stds(mapdl): mapdl._read_stds() assert mapdl._stdout is not None assert mapdl._stderr is not None + + +def test_exception_message_length(monkeypatch, mapdl): + channel = grpc.insecure_channel( + mapdl._channel_str, + options=[ + ("grpc.max_receive_message_length", int(1 * 1024)), + ], + ) + mapdl = MapdlGrpc(channel=channel) + assert mapdl.is_alive + + with pytest.raises(MapdlgRPCError, match="Received message larger than max"): + mapdl.prep7() + mapdl.dim("myarr", "", 1e4) + mapdl.vfill("myarr", "rand", 0, 1) # filling array with random numbers + + # Retrieving + values = mapdl.parameters["myarr"] + + assert mapdl.is_alive + + +def test_generic_grpc_exception(monkeypatch, grpc_channel): + mapdl = MapdlGrpc(channel=grpc_channel) + assert mapdl.is_alive + + class UnavailableError(grpc.RpcError): + def __init__(self, message="Service is temporarily unavailable."): + self._message = message + self._code = grpc.StatusCode.UNAVAILABLE + super().__init__(message) + + def code(self): + return self._code + + def details(self): + return self._message + + def _raise_error_code(args, **kwargs): + raise UnavailableError() + + monkeypatch.setattr(mapdl._stub, "SendCommand", _raise_error_code) + + with pytest.raises( + MapdlExitedError, match="MAPDL server connection terminated unexpectedly while" + ): + mapdl.prep7() + + assert mapdl.is_alive From de4e9e510fa91da8fb316e140c0750e60dcb26b5 Mon Sep 17 00:00:00 2001 From: German <28149841+germa89@users.noreply.github.com> Date: Wed, 31 Jul 2024 10:47:53 +0200 Subject: [PATCH 07/12] fix: test --- tests/test_grpc.py | 19 ++++++++++++------- 1 file changed, 12 insertions(+), 7 deletions(-) diff --git a/tests/test_grpc.py b/tests/test_grpc.py index 1687ee3a6f..208a80b2f3 100644 --- a/tests/test_grpc.py +++ b/tests/test_grpc.py @@ -582,18 +582,23 @@ def test_exception_message_length(monkeypatch, mapdl): ("grpc.max_receive_message_length", int(1 * 1024)), ], ) - mapdl = MapdlGrpc(channel=channel) - assert mapdl.is_alive + mapdl2 = MapdlGrpc(channel=channel) + assert mapdl2.is_alive with pytest.raises(MapdlgRPCError, match="Received message larger than max"): - mapdl.prep7() - mapdl.dim("myarr", "", 1e4) - mapdl.vfill("myarr", "rand", 0, 1) # filling array with random numbers + mapdl2.prep7() + mapdl2.dim("myarr", "", 1e4) + mapdl2.vfill("myarr", "rand", 0, 1) # filling array with random numbers # Retrieving - values = mapdl.parameters["myarr"] + values = mapdl2.parameters["myarr"] - assert mapdl.is_alive + assert mapdl2.is_alive + + # Deleting generated mapdl instance and channel. + channel.close() + mapdl._exited = True # To avoid side effects. + mapdl2.exit() def test_generic_grpc_exception(monkeypatch, grpc_channel): From 223f2256e6d44b706c40e31cfd096d3c1bec8826 Mon Sep 17 00:00:00 2001 From: German <28149841+germa89@users.noreply.github.com> Date: Wed, 31 Jul 2024 12:26:52 +0200 Subject: [PATCH 08/12] fix: test by reducing the message length limit --- tests/test_grpc.py | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/tests/test_grpc.py b/tests/test_grpc.py index 208a80b2f3..dd95211104 100644 --- a/tests/test_grpc.py +++ b/tests/test_grpc.py @@ -579,18 +579,18 @@ def test_exception_message_length(monkeypatch, mapdl): channel = grpc.insecure_channel( mapdl._channel_str, options=[ - ("grpc.max_receive_message_length", int(1 * 1024)), + ("grpc.max_receive_message_length", int(0.5 * 1024)), ], ) mapdl2 = MapdlGrpc(channel=channel) assert mapdl2.is_alive - with pytest.raises(MapdlgRPCError, match="Received message larger than max"): - mapdl2.prep7() - mapdl2.dim("myarr", "", 1e4) - mapdl2.vfill("myarr", "rand", 0, 1) # filling array with random numbers + mapdl2.prep7() + mapdl2.dim("myarr", "", 1e5) + mapdl2.vfill("myarr", "rand", 0, 1) # filling array with random numbers - # Retrieving + # Retrieving + with pytest.raises(MapdlgRPCError, match="Received message larger than max"): values = mapdl2.parameters["myarr"] assert mapdl2.is_alive From 1159f0752300ad8148835fd0ded1ad817b9e4216 Mon Sep 17 00:00:00 2001 From: German <28149841+germa89@users.noreply.github.com> Date: Wed, 31 Jul 2024 13:10:28 +0200 Subject: [PATCH 09/12] fix:tests --- tests/test_grpc.py | 15 +++++++++------ 1 file changed, 9 insertions(+), 6 deletions(-) diff --git a/tests/test_grpc.py b/tests/test_grpc.py index dd95211104..1e22896369 100644 --- a/tests/test_grpc.py +++ b/tests/test_grpc.py @@ -141,11 +141,14 @@ def test_clear_multiple(mapdl): mapdl.run("/CLEAR") -@pytest.mark.xfail( - reason="MAPDL bug 867421", raises=(MapdlExitedError, UnicodeDecodeError) -) def test_invalid_get_bug(mapdl): - with pytest.raises((MapdlRuntimeError, MapdlCommandIgnoredError)): + # versions before 24.1 should raise an error + if mapdl.version < 24.1: + context = pytest.raises((MapdlRuntimeError, MapdlCommandIgnoredError)) + else: + context = NullContext + + with context: mapdl.get_value("ACTIVE", item1="SET", it1num="invalid") @@ -579,7 +582,7 @@ def test_exception_message_length(monkeypatch, mapdl): channel = grpc.insecure_channel( mapdl._channel_str, options=[ - ("grpc.max_receive_message_length", int(0.5 * 1024)), + ("grpc.max_receive_message_length", int(1024)), ], ) mapdl2 = MapdlGrpc(channel=channel) @@ -597,7 +600,7 @@ def test_exception_message_length(monkeypatch, mapdl): # Deleting generated mapdl instance and channel. channel.close() - mapdl._exited = True # To avoid side effects. + mapdl2._exited = True # To avoid side effects. mapdl2.exit() From 9c41f1ca6ddf520766dbe1148a496aeef86aacc9 Mon Sep 17 00:00:00 2001 From: German <28149841+germa89@users.noreply.github.com> Date: Wed, 31 Jul 2024 13:28:23 +0200 Subject: [PATCH 10/12] fix:tests --- tests/test_grpc.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/tests/test_grpc.py b/tests/test_grpc.py index 1e22896369..0bc7cd32bb 100644 --- a/tests/test_grpc.py +++ b/tests/test_grpc.py @@ -144,9 +144,9 @@ def test_clear_multiple(mapdl): def test_invalid_get_bug(mapdl): # versions before 24.1 should raise an error if mapdl.version < 24.1: - context = pytest.raises((MapdlRuntimeError, MapdlCommandIgnoredError)) + context = pytest.raises(MapdlCommandIgnoredError) else: - context = NullContext + context = pytest.raises((MapdlRuntimeError, MapdlCommandIgnoredError)) with context: mapdl.get_value("ACTIVE", item1="SET", it1num="invalid") From 6f566d72cc75d11ffd9b45d275059c1770fafaf3 Mon Sep 17 00:00:00 2001 From: German <28149841+germa89@users.noreply.github.com> Date: Wed, 31 Jul 2024 13:47:06 +0200 Subject: [PATCH 11/12] fix: skip tests if not on remote --- tests/conftest.py | 8 ++++++++ tests/test_grpc.py | 4 +++- 2 files changed, 11 insertions(+), 1 deletion(-) diff --git a/tests/conftest.py b/tests/conftest.py index c91df8b2a7..8cfaa8095c 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -87,6 +87,11 @@ reason="Skipping because not on local. ", ) +skip_if_not_remote = pytest.mark.skipif( + ON_LOCAL, + reason="Skipping because not on remote. ", +) + skip_if_on_cicd = pytest.mark.skipif( ON_CI, reason="""Skip if on CI/CD.""", @@ -146,6 +151,9 @@ def requires(requirement: str): elif "local" == requirement: return skip_if_not_local + elif "remote" == requirement: + return skip_if_not_remote + elif "cicd" == requirement: return skip_if_on_cicd diff --git a/tests/test_grpc.py b/tests/test_grpc.py index 0bc7cd32bb..a9916d8695 100644 --- a/tests/test_grpc.py +++ b/tests/test_grpc.py @@ -578,7 +578,9 @@ def test__check_stds(mapdl): assert mapdl._stderr is not None -def test_exception_message_length(monkeypatch, mapdl): +@requires("remote") +def test_exception_message_length(mapdl): + # This test does not fail if running on local channel = grpc.insecure_channel( mapdl._channel_str, options=[ From 42c0eb1e5353e5217928b41ddeb9e7bc3870c110 Mon Sep 17 00:00:00 2001 From: German <28149841+germa89@users.noreply.github.com> Date: Thu, 8 Aug 2024 19:20:41 +0200 Subject: [PATCH 12/12] fix: format and wrong unpacking --- src/ansys/mapdl/core/errors.py | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/src/ansys/mapdl/core/errors.py b/src/ansys/mapdl/core/errors.py index d5f5df9f68..119446a66a 100644 --- a/src/ansys/mapdl/core/errors.py +++ b/src/ansys/mapdl/core/errors.py @@ -350,7 +350,7 @@ def handle_generic_grpc_error(error, func, args, kwargs): # can't use isinstance here due to circular imports try: class_name = args[0].__class__.__name__ - except: + except (IndexError, AttributeError): class_name = "" # trying to get "cmd" argument: @@ -360,9 +360,9 @@ def handle_generic_grpc_error(error, func, args, kwargs): caller = func.__name__ if cmd: - msg_ = f"running:\n\t{cmd}\ncalled by:\n\t{caller}\n" + msg_ = f"running:\n {cmd}\ncalled by:\n {caller}\n" else: - msg_ = f"calling:{caller}\nwith the following arguments:\nargs: {list(*args)}\nkwargs: {list(**kwargs_)}" + msg_ = f"calling:{caller}\nwith the following arguments:\n args: {args}\n kwargs: {kwargs}" if class_name == "MapdlGrpc": mapdl = args[0] @@ -370,9 +370,9 @@ def handle_generic_grpc_error(error, func, args, kwargs): mapdl = args[0]._mapdl msg = ( - f"MAPDL server connection terminated unexpectedly while {msg_}\n" + f"Error:\nMAPDL server connection terminated unexpectedly while {msg_}\n" "Error:\n" - f"\t{error.details()}\n" + f" {error.details()}\n" f"Full error:\n{error}" )