Skip to content

Py/client: re-open session via rpc if id is provided, fallback to create new#160

Merged
hon-gyu merged 2 commits into
mainfrom
hy/reopen-session
May 29, 2026
Merged

Py/client: re-open session via rpc if id is provided, fallback to create new#160
hon-gyu merged 2 commits into
mainfrom
hy/reopen-session

Conversation

@hon-gyu

@hon-gyu hon-gyu commented May 26, 2026

Copy link
Copy Markdown
Contributor

closes #159 on client side

Note: #159 points out a server side issue that the session not found error is returned untyped (as GeneraIOError) containing ocaml trace. I think this should be fixed as well

Comment thread src/py/client/_common.py
Comment on lines +14 to +24
def is_session_not_found(ex: TwirpServerException) -> bool:
# TODO:
# The server currently surfaces an expired/missing session as a generic
# internal error; the only stable signal is the substring in the body
# message. The phrasing differs between RPCs:
# - SessionManager.open_session -> "Unknown session"
# - other session-bearing RPCs -> "Session not found"
# Replace with a typed code check once the server is updated.
body = (getattr(ex, "meta", None) or {}).get("body") or {} # type: ignore
msg = body.get("msg") or "" # type: ignore
return "Session not found" in msg or "Unknown session" in msg

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

This is a temporary workaround because the server currently surfaces an expired/missing session as a generic internal error; the only stable signal is the substring in the body message.

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.

Yep, this needs a server-side fix as well, I'll add a new error kind for it.

@hon-gyu hon-gyu force-pushed the hy/reopen-session branch from e442d34 to 50d9320 Compare May 26, 2026 10:09
@hon-gyu hon-gyu requested a review from wintersteiger May 26, 2026 10:10
Comment thread src/py/client/_async.py Outdated
@hon-gyu hon-gyu requested a review from wintersteiger May 28, 2026 20:44

@wintersteiger wintersteiger left a comment

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.

LGTM, many thanks! The new error kind is merged here and in imandrax (dev), so we could start checking for that one any time now too.

@hon-gyu hon-gyu merged commit f29eb34 into main May 29, 2026
6 checks passed
@hon-gyu hon-gyu deleted the hy/reopen-session branch May 29, 2026 14:28
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.

Py/client: reattaching to an ended session yields "Session not found" on first RPC

2 participants