Skip to content

Fix invalid session bad error description #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

Draft
zoechanzy wants to merge 2 commits into
base: master
Choose a base branch
from
+140 −0
Draft

Fix invalid session bad error description #2181

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 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
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
10 changes: 10 additions & 0 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 @@ -20,12 +20,22 @@
pass
% if 'error_message' in config['functions']:

try:
'''
It is possible that the session is valid but the returned_error_code unequal to error_code
'''
error_string = self.error_message(error_code)
return error_string
except errors.Error:
pass

try:
'''

@bkeryan bkeryan Jun 29, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

FYI, it's weird that these are multi-line string literals instead of comments. They are not in a position where they will be interpreted as docstrings.

@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.

Yeah, that struck me as odd, too.
Apparently the Python interpreter will evaluate it as a string and then discard it?

@bkeryan bkeryan Jun 29, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Yes, it will.

BTW it's legal to write a docstring inside an init method, e.g. https://www.sphinx-doc.org/en/master/usage/extensions/example_google.html

self.attr5 = None
"""str: Docstring *after* attribute, with type specified."""

But that's not what's being done here.

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:
Expand Down
10 changes: 10 additions & 0 deletions generated/nidcpower/nidcpower/_library_interpreter.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -103,12 +103,22 @@ def get_error_description(self, error_code):
except errors.Error:
pass

try:
'''
It is possible that the session is valid but the returned_error_code unequal to error_code
'''
error_string = self.error_message(error_code)
return error_string
except errors.Error:
pass
Comment on lines +110 to +113

@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.


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 +121 to 122

@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.

My current approach is wrong because it reset valid session. Adding error_message call will not help to protect against it because it is unable to know error_message throw exception because of invalid session or any other internal error. Maybe I should compare its error code directly with the invalid error session code?

return error_string
except errors.Error:
Expand Down
10 changes: 10 additions & 0 deletions generated/nidigital/nidigital/_library_interpreter.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -101,12 +101,22 @@ def get_error_description(self, error_code):
except errors.Error:
pass

try:
'''
It is possible that the session is valid but the returned_error_code unequal to error_code
'''
error_string = self.error_message(error_code)
return error_string
except errors.Error:
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.
'''
self.set_session_handle()
error_string = self.error_message(error_code)
return error_string
except errors.Error:
Expand Down
10 changes: 10 additions & 0 deletions generated/nidmm/nidmm/_library_interpreter.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -99,12 +99,22 @@ def get_error_description(self, error_code):
except errors.Error:
pass

try:
'''
It is possible that the session is valid but the returned_error_code unequal to error_code
'''
error_string = self.error_message(error_code)
return error_string
except errors.Error:
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.
'''
self.set_session_handle()
error_string = self.error_message(error_code)
return error_string
except errors.Error:
Expand Down
10 changes: 10 additions & 0 deletions generated/nifake/nifake/_library_interpreter.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -115,12 +115,22 @@ def get_error_description(self, error_code):
except errors.Error:
pass

try:
'''
It is possible that the session is valid but the returned_error_code unequal to error_code
'''
error_string = self.error_message(error_code)
return error_string
except errors.Error:
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.
'''
self.set_session_handle()
error_string = self.error_message(error_code)
return error_string
except errors.Error:
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
10 changes: 10 additions & 0 deletions generated/nifgen/nifgen/_library_interpreter.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -99,12 +99,22 @@ def get_error_description(self, error_code):
except errors.Error:
pass

try:
'''
It is possible that the session is valid but the returned_error_code unequal to error_code
'''
error_string = self.error_message(error_code)
return error_string
except errors.Error:
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.
'''
self.set_session_handle()
error_string = self.error_message(error_code)
return error_string
except errors.Error:
Expand Down
10 changes: 10 additions & 0 deletions generated/niscope/niscope/_library_interpreter.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -103,12 +103,22 @@ def get_error_description(self, error_code):
except errors.Error:
pass

try:
'''
It is possible that the session is valid but the returned_error_code unequal to error_code
'''
error_string = self.error_message(error_code)
return error_string
except errors.Error:
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.
'''
self.set_session_handle()
error_string = self.error_message(error_code)
return error_string
except errors.Error:
Expand Down
10 changes: 10 additions & 0 deletions generated/niswitch/niswitch/_library_interpreter.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -99,12 +99,22 @@ def get_error_description(self, error_code):
except errors.Error:
pass

try:
'''
It is possible that the session is valid but the returned_error_code unequal to error_code
'''
error_string = self.error_message(error_code)
return error_string
except errors.Error:
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.
'''
self.set_session_handle()
error_string = self.error_message(error_code)
return error_string
except errors.Error:
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