Skip to content

fix: SMPClient context logs formatted traceback, not repr#64

Merged
JPHutchins merged 1 commit intointercreate:mainfrom
GHF:fix/smpclient-context-traceback
May 17, 2025
Merged

fix: SMPClient context logs formatted traceback, not repr#64
JPHutchins merged 1 commit intointercreate:mainfrom
GHF:fix/smpclient-context-traceback

Conversation

@GHF
Copy link
Contributor

@GHF GHF commented May 2, 2025

Previously, exiting the SMPClient async context with an exception results in logging f"{traceback=}" whose output looks like:

traceback=<traceback object at 0x7f4c3fea2980>

as part of a full log message like so:

Exception in SMPClient: exc_type=<class 'smpclient.transport.SMPTransportDisconnected'>, exc_value=SMPTransportDisconnected('SmpNativeTransport cancelled receive due to disconnection'), traceback=<traceback object at 0x7f4c3fea2980>

Instead, format the exception using traceback.format_exc() (which also grabs the originating exception with sys.exception()). Now the output looks like:

  Exception in SMPClient:
  Traceback (most recent call last):
    File "<string>", line 11, in run_echo_test
    File "/opt/axe/develop/latest/desktop/sysroots/x86-64-v3-poky-linux/usr/lib/python3.12/site-packages/smpclient/__init__.py", line 160, in request
      frame = await self._transport.send_and_receive(request.BYTES)
              ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
    File "<string>", line 67, in send_and_receive
    File "<string>", line 62, in receive
  smpclient.transport.SMPTransportDisconnected: SmpNativeTransport cancelled receive due to disconnection

Previously, exiting the SMPClient async context with an exception
results in logging f"{traceback=}" whose output looks like:

  traceback=<traceback object at 0x7f4c3fea2980>

as part of a full log message like so:

  Exception in SMPClient: exc_type=<class 'smpclient.transport.SMPTransportDisconnected'>, exc_value=SMPTransportDisconnected('SmpNativeTransport cancelled receive due to disconnection'), traceback=<traceback object at 0x7f4c3fea2980>

Instead, format the exception using traceback.format_exc() (which also
grabs the originating exception with sys.exception()). Now the output
looks like:

  Exception in SMPClient:
  Traceback (most recent call last):
    File "<string>", line 11, in run_echo_test
    File "/opt/axe/develop/latest/desktop/sysroots/x86-64-v3-poky-linux/usr/lib/python3.12/site-packages/smpclient/__init__.py", line 160, in request
      frame = await self._transport.send_and_receive(request.BYTES)
              ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
    File "<string>", line 67, in send_and_receive
    File "<string>", line 62, in receive
  smpclient.transport.SMPTransportDisconnected: SmpNativeTransport cancelled receive due to disconnection
Copy link
Collaborator

@JPHutchins JPHutchins left a comment

Choose a reason for hiding this comment

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

Thanks! This is a great improvement. Getting good exceptions from asyncio is always challenging, and I'm not certain of best practices. What do you think about raising the exception after the disconnect so that users can catch them?

I read through this conversation recently, but I'm not sure that I absorbed it!

https://discuss.python.org/t/asyncio-tasks-and-exception-handling-recommended-idioms/23806

Copy link
Collaborator

@JPHutchins JPHutchins left a comment

Choose a reason for hiding this comment

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

I've looked into this and it seems like suppressing exceptions is the expected behavior for async context managers. The exception cannot be (easily) retrieved outside of the block because the context has ended.

So, logging will have to do for now!

@JPHutchins JPHutchins merged commit 2877523 into intercreate:main May 17, 2025
25 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants