Skip to content

Fix the python session to return correct error message when invalid session for MI drivers #2181

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
zoechanzy wants to merge 6 commits into
base: master
Choose a base branch
from
+220 −40
Open

Fix the python session to return correct error message when invalid session for MI drivers #2181

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
82373f8
Added self.set_session_handle()
zoechanzy Jun 29, 2026
ab14a93
Add unit test
zoechanzy Jun 29, 2026
5788750
Use comment instead of docstring
zoechanzy Jul 2, 2026
4b16c37
Update get_error_description
zoechanzy Jul 2, 2026
d239d81
Save session before next try block
zoechanzy Jul 2, 2026
dfb69f2
Update comments
zoechanzy Jul 2, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
25 changes: 20 additions & 5 deletions build/templates/_library_interpreter.py/_get_error_description.py.mako
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,14 +21,29 @@
% if 'error_message' in config['functions']:

try:
'''
It is expected for get_error to raise when the session is invalid
(IVI spec requires GetError to fail).
Use error_message instead. It doesn't require a session.
'''
# get_error reads the session's error queue, which may have been overwritten,
# causing it to return a mismatched error code. error_message takes the error
# code directly as a parameter and looks up its description without reading the
# queue, it will return the description for the specific error code.
# Use error_message in current session before the handle reset in the next block.

error_string = self.error_message(error_code)
return error_string
except errors.Error:
pass

save_vi = self.get_session_handle()
try:
# It is expected for get_error to raise when the session is invalid
# (IVI spec requires GetError to fail).
# Use error_message instead. It doesn't require a session.

self.set_session_handle()
error_string = self.error_message(error_code)
return error_string
except errors.Error:
pass
finally:
self.set_session_handle(save_vi)
% endif
return "Failed to retrieve error description."
25 changes: 20 additions & 5 deletions generated/nidcpower/nidcpower/_library_interpreter.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -104,15 +104,30 @@ def get_error_description(self, error_code):
pass

try:
'''
It is expected for get_error to raise when the session is invalid
(IVI spec requires GetError to fail).
Use error_message instead. It doesn't require a session.
'''
# get_error reads the session's error queue, which may have been overwritten,
# causing it to return a mismatched error code. error_message takes the error
# code directly as a parameter and looks up its description without reading the
# queue, it will return the description for the specific error code.
# Use error_message in current session before the handle reset in the next block.

error_string = self.error_message(error_code)
return error_string
except errors.Error:
pass
Comment on lines +113 to +116

@ni-jfitzger ni-jfitzger Jun 29, 2026

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It is possible that the session is valid but the returned_error_code unequal to error_code

If this happens, then something likely went wrong retrieving the error message. I'm not sure calling error_message instead of get_error_description is going to change anything in that case.

@zoechanzy zoechanzy Jun 30, 2026

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Calling get_error_description again might cause infinite loop.

@ni-jfitzger ni-jfitzger Jun 30, 2026 •
edited
Loading

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No, I wasn't suggesting that we call get_error_description again. I'm merely trying to think about what behavior we'll see if this new section of code executes.

Oh, i see. Sorry, I meant get_error, not get_error_description.

@jfan-1995 jfan-1995 Jul 2, 2026 •
edited
Loading

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I also find this comment a bit confusing....

What I understand is get_error reads what is currently in the session, if it is overwritten or cleared it could still successfully run but will return mismatched error code.

So this try block is to check the error_message in the current session first before proceed to the set_session_handle in the next try block.

@jfan-1995 jfan-1995 Jul 2, 2026 •
edited
Loading

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

How about wording the comment to be like this?

# get_error reads the session's error queue, which may have been overwritten,
# causing it to return a mismatched error code. error_message takes the error
# code directly as a parameter and looks up its description without reading the
# queue, so it will just return the description for the specific error code.
# Try it here with the current session before the handle reset in the next block.


save_vi = self.get_session_handle()
try:
# It is expected for get_error to raise when the session is invalid
# (IVI spec requires GetError to fail).
# Use error_message instead. It doesn't require a session.

self.set_session_handle()
error_string = self.error_message(error_code)
Comment on lines +124 to 125

@ni-jfitzger ni-jfitzger Jun 29, 2026

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Did you add the other error_message call to protect against resetting the session handle in instances where the session is valid but we fail to retrieve the error message?

@zoechanzy zoechanzy Jun 30, 2026 •
edited
Loading

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If the scenario is valid session but failed to retrieve error message, it will enter third try block. Resetting the session will not affect its error_code so it will flow into the error_string = self.error_message(error_code) as well. It will still fail to retrieve the error message since the error_code is unchanged. So, it will raise execption and return "Failed to retrieve error description"

If the scenario is invalid session, it will reset the session in third try block and self.error_message(error_code) will not raise execption because session is set to 0 already. So, it will return correct "The session handle is invalid" message.

@ni-jfitzger ni-jfitzger Jun 30, 2026 •
edited
Loading

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If the scenario is valid session but failed to retrieve error message, it will enter third try block.

I'm not sure that's true.
I think the 2nd try block will return from get_error_description with an empty error string.
Have you done testing to determine the behavior?

If you believe we could enter the third try block and reset the session when the session is valid and we fail to retrieve the error message, then you shouldn't be resetting the session. Failure to retrieve an error message does not justify leaking a valid session handle.

@zoechanzy zoechanzy Jul 1, 2026 •
edited
Loading

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think the 2nd try block will return from get_error_description with an empty error string.
Have you done testing to determine the behavior?

I tested with :

  • Unknown error code (-9999999999) → get_error_description returned "Explanation could not be found for the requested status code."
  • VISA layer error code (-1073807339 )→ get_error_description returned "VI_ERROR_TMO: Timeout expired before operation completed."

Since the real driver does not raise errors.Error from error_message() on a valid session either, I used a mock to cover the except errors.Error in second try block and third try block.
with patch.object( type(session._interpreter), 'error_message', side_effect=errors.Error(real_error_code) ): description = session._interpreter.get_error_description(real_error_code)

It returns "Failed to retrieve error description."

Failure to retrieve an error message does not justify leaking a valid session handle.

I will update code to save the session handle before resetting it and restore it afterward to avoid leaking a valid session handle. We cannot selectively restore only valid sessions because we can only determine if a session handle is valid by calling a C driver function and observe whether it succeeds or fails.

return error_string
except errors.Error:
pass
finally:
self.set_session_handle(save_vi)
return "Failed to retrieve error description."

def abort(self, channel_name): # noqa: N802
Expand Down
25 changes: 20 additions & 5 deletions generated/nidigital/nidigital/_library_interpreter.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -102,15 +102,30 @@ def get_error_description(self, error_code):
pass

try:
'''
It is expected for get_error to raise when the session is invalid
(IVI spec requires GetError to fail).
Use error_message instead. It doesn't require a session.
'''
# get_error reads the session's error queue, which may have been overwritten,
# causing it to return a mismatched error code. error_message takes the error
# code directly as a parameter and looks up its description without reading the
# queue, it will return the description for the specific error code.
# Use error_message in current session before the handle reset in the next block.

error_string = self.error_message(error_code)
return error_string
except errors.Error:
pass

save_vi = self.get_session_handle()
try:
# It is expected for get_error to raise when the session is invalid
# (IVI spec requires GetError to fail).
# Use error_message instead. It doesn't require a session.

self.set_session_handle()
error_string = self.error_message(error_code)
return error_string
except errors.Error:
pass
finally:
self.set_session_handle(save_vi)
return "Failed to retrieve error description."

def abort(self): # noqa: N802
Expand Down
25 changes: 20 additions & 5 deletions generated/nidmm/nidmm/_library_interpreter.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -100,15 +100,30 @@ def get_error_description(self, error_code):
pass

try:
'''
It is expected for get_error to raise when the session is invalid
(IVI spec requires GetError to fail).
Use error_message instead. It doesn't require a session.
'''
# get_error reads the session's error queue, which may have been overwritten,
# causing it to return a mismatched error code. error_message takes the error
# code directly as a parameter and looks up its description without reading the
# queue, it will return the description for the specific error code.
# Use error_message in current session before the handle reset in the next block.

error_string = self.error_message(error_code)
return error_string
except errors.Error:
pass

save_vi = self.get_session_handle()
try:
# It is expected for get_error to raise when the session is invalid
# (IVI spec requires GetError to fail).
# Use error_message instead. It doesn't require a session.

self.set_session_handle()
error_string = self.error_message(error_code)
return error_string
except errors.Error:
pass
finally:
self.set_session_handle(save_vi)
return "Failed to retrieve error description."

def abort(self): # noqa: N802
Expand Down
25 changes: 20 additions & 5 deletions generated/nifake/nifake/_library_interpreter.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -116,15 +116,30 @@ def get_error_description(self, error_code):
pass

try:
'''
It is expected for get_error to raise when the session is invalid
(IVI spec requires GetError to fail).
Use error_message instead. It doesn't require a session.
'''
# get_error reads the session's error queue, which may have been overwritten,
# causing it to return a mismatched error code. error_message takes the error
# code directly as a parameter and looks up its description without reading the
# queue, it will return the description for the specific error code.
# Use error_message in current session before the handle reset in the next block.

error_string = self.error_message(error_code)
return error_string
except errors.Error:
pass

save_vi = self.get_session_handle()
try:
# It is expected for get_error to raise when the session is invalid
# (IVI spec requires GetError to fail).
# Use error_message instead. It doesn't require a session.

self.set_session_handle()
error_string = self.error_message(error_code)
return error_string
except errors.Error:
pass
finally:
self.set_session_handle(save_vi)
return "Failed to retrieve error description."

def abort(self): # noqa: N802
Expand Down
30 changes: 30 additions & 0 deletions generated/nifake/nifake/unit_tests/test_library_interpreter.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -716,6 +716,36 @@ def test_get_error_description_error_message_error(self):
assert e.description == test_error_desc
self.patched_library.niFake_error_message.assert_called_once_with(_matchers.ViSessionMatcher(SESSION_NUM_FOR_TEST), _matchers.ViInt32Matcher(test_error_code), _matchers.ViCharBufferMatcher(256))

def test_get_error_description_error_message_after_session_reset(self):
test_error_code = -42
test_error_desc = "The answer to the ultimate question"
self.patched_library.niFake_PoorlyNamedSimpleFunction.side_effect = self.side_effects_helper.niFake_PoorlyNamedSimpleFunction
self.side_effects_helper['PoorlyNamedSimpleFunction']['return'] = test_error_code
self.patched_library.niFake_GetError.side_effect = self.side_effects_helper.niFake_GetError
self.side_effects_helper['GetError']['errorCode'] = -1
self.side_effects_helper['GetError']['description'] = "Shouldn't get this"
self.side_effects_helper['GetError']['return'] = -2
self.side_effects_helper['error_message']['errorMessage'] = test_error_desc

def error_message_side_effect(vi, error_code, error_message_buf):
if vi.value == SESSION_NUM_FOR_TEST:
return -3
return self.side_effects_helper.niFake_error_message(vi, error_code, error_message_buf)

self.patched_library.niFake_error_message.side_effect = error_message_side_effect
interpreter = self.get_initialized_library_interpreter()
try:
interpreter.simple_function()
assert False
except nifake.Error as e:
assert e.code == test_error_code
assert e.description == test_error_desc
assert self.patched_library.niFake_error_message.call_count == 2
self.patched_library.niFake_error_message.assert_has_calls([
call(_matchers.ViSessionMatcher(SESSION_NUM_FOR_TEST), _matchers.ViInt32Matcher(test_error_code), _matchers.ViCharBufferMatcher(256)),
call(_matchers.ViSessionMatcher(0), _matchers.ViInt32Matcher(test_error_code), _matchers.ViCharBufferMatcher(256)),
])

# Custom types

def test_set_custom_type(self):
Expand Down
25 changes: 20 additions & 5 deletions generated/nifgen/nifgen/_library_interpreter.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -100,15 +100,30 @@ def get_error_description(self, error_code):
pass

try:
'''
It is expected for get_error to raise when the session is invalid
(IVI spec requires GetError to fail).
Use error_message instead. It doesn't require a session.
'''
# get_error reads the session's error queue, which may have been overwritten,
# causing it to return a mismatched error code. error_message takes the error
# code directly as a parameter and looks up its description without reading the
# queue, it will return the description for the specific error code.
# Use error_message in current session before the handle reset in the next block.

error_string = self.error_message(error_code)
return error_string
except errors.Error:
pass

save_vi = self.get_session_handle()
try:
# It is expected for get_error to raise when the session is invalid
# (IVI spec requires GetError to fail).
# Use error_message instead. It doesn't require a session.

self.set_session_handle()
error_string = self.error_message(error_code)
return error_string
except errors.Error:
pass
finally:
self.set_session_handle(save_vi)
return "Failed to retrieve error description."

def abort(self): # noqa: N802
Expand Down
25 changes: 20 additions & 5 deletions generated/niscope/niscope/_library_interpreter.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -104,15 +104,30 @@ def get_error_description(self, error_code):
pass

try:
'''
It is expected for get_error to raise when the session is invalid
(IVI spec requires GetError to fail).
Use error_message instead. It doesn't require a session.
'''
# get_error reads the session's error queue, which may have been overwritten,
# causing it to return a mismatched error code. error_message takes the error
# code directly as a parameter and looks up its description without reading the
# queue, it will return the description for the specific error code.
# Use error_message in current session before the handle reset in the next block.

error_string = self.error_message(error_code)
return error_string
except errors.Error:
pass

save_vi = self.get_session_handle()
try:
# It is expected for get_error to raise when the session is invalid
# (IVI spec requires GetError to fail).
# Use error_message instead. It doesn't require a session.

self.set_session_handle()
error_string = self.error_message(error_code)
return error_string
except errors.Error:
pass
finally:
self.set_session_handle(save_vi)
return "Failed to retrieve error description."

def abort(self): # noqa: N802
Expand Down
25 changes: 20 additions & 5 deletions generated/niswitch/niswitch/_library_interpreter.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -100,15 +100,30 @@ def get_error_description(self, error_code):
pass

try:
'''
It is expected for get_error to raise when the session is invalid
(IVI spec requires GetError to fail).
Use error_message instead. It doesn't require a session.
'''
# get_error reads the session's error queue, which may have been overwritten,
# causing it to return a mismatched error code. error_message takes the error
# code directly as a parameter and looks up its description without reading the
# queue, it will return the description for the specific error code.
# Use error_message in current session before the handle reset in the next block.

error_string = self.error_message(error_code)
return error_string
except errors.Error:
pass

save_vi = self.get_session_handle()
try:
# It is expected for get_error to raise when the session is invalid
# (IVI spec requires GetError to fail).
# Use error_message instead. It doesn't require a session.

self.set_session_handle()
error_string = self.error_message(error_code)
return error_string
except errors.Error:
pass
finally:
self.set_session_handle(save_vi)
return "Failed to retrieve error description."

def abort(self): # noqa: N802
Expand Down
30 changes: 30 additions & 0 deletions src/nifake/unit_tests/test_library_interpreter.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -716,6 +716,36 @@ def test_get_error_description_error_message_error(self):
assert e.description == test_error_desc
self.patched_library.niFake_error_message.assert_called_once_with(_matchers.ViSessionMatcher(SESSION_NUM_FOR_TEST), _matchers.ViInt32Matcher(test_error_code), _matchers.ViCharBufferMatcher(256))

def test_get_error_description_error_message_after_session_reset(self):
test_error_code = -42
test_error_desc = "The answer to the ultimate question"
self.patched_library.niFake_PoorlyNamedSimpleFunction.side_effect = self.side_effects_helper.niFake_PoorlyNamedSimpleFunction
self.side_effects_helper['PoorlyNamedSimpleFunction']['return'] = test_error_code
self.patched_library.niFake_GetError.side_effect = self.side_effects_helper.niFake_GetError
self.side_effects_helper['GetError']['errorCode'] = -1
self.side_effects_helper['GetError']['description'] = "Shouldn't get this"
self.side_effects_helper['GetError']['return'] = -2
self.side_effects_helper['error_message']['errorMessage'] = test_error_desc

def error_message_side_effect(vi, error_code, error_message_buf):
if vi.value == SESSION_NUM_FOR_TEST:
return -3
return self.side_effects_helper.niFake_error_message(vi, error_code, error_message_buf)

self.patched_library.niFake_error_message.side_effect = error_message_side_effect
interpreter = self.get_initialized_library_interpreter()
try:
interpreter.simple_function()
assert False
except nifake.Error as e:
assert e.code == test_error_code
assert e.description == test_error_desc
assert self.patched_library.niFake_error_message.call_count == 2
self.patched_library.niFake_error_message.assert_has_calls([
call(_matchers.ViSessionMatcher(SESSION_NUM_FOR_TEST), _matchers.ViInt32Matcher(test_error_code), _matchers.ViCharBufferMatcher(256)),
call(_matchers.ViSessionMatcher(0), _matchers.ViInt32Matcher(test_error_code), _matchers.ViCharBufferMatcher(256)),
])

# Custom types

def test_set_custom_type(self):
Expand Down
Loading