client, test/suites: forward session timeout cause over websocket by shardool-patil · Pull Request #1519 · canonical/microcloud

shardool-patil · 2026-08-21T09:51:03Z

Description

When a trust establishment session expires, the client previously received a generic connection error (websocket: close 1006: unexpected EOF) instead of the actual cause (Session timeout exceeded)[cite: 5].

Changes

client/websocket.go:
- Updated WriteClose to write ControlClose directly over the WebSocket under writeLock so close frames are sent even when gwCtx is canceled on timeout[cite: 5].
- Updated ReceiveWithContext to check channel closure and return context.Cause(w.ctx)[cite: 5].
test/suites/basic.sh:
- Added a test case in test_interactive with SESSION_TIMEOUT="5" verifying that both the initiator and joiner receive "Session timeout exceeded"[cite: 2].

Checklist

I have read the contributing guidelines and attest that all commits in this PR are signed off, cryptographically signed, and follow this project's commit structure.
I have checked and added or updated relevant documentation.

Related issues

Fixes Session expiry isn't forwarded to the client #1245

@roosterfish i have taken into consideration the insights you provided and resolved the issue, so take a look at it when you get time.

roosterfish

Thanks for taking a look at this, please see my comments on the test.

roosterfish · 2026-08-21T10:08:21Z

+  echo "Initiate a MicroCloud cluster with a short session timeout and verify the joiner receives the timeout error"
+  export MULTI_NODE="yes"
+  export LOOKUP_IFACE="enp5s0"
+  export EXPECT_PEERS=1


I guess what you would need here is export EXPECT_PEERS=2, then let only micro02 attempt to join. As micro01 is the initiator, it will wait for two peers, but only micro02 reaches out so it will cause the session to expire.

roosterfish · 2026-08-21T10:08:39Z

    validate_system_microovn ${m}
  done
-}
+}


Newline missing.

roosterfish · 2026-08-21T10:10:27Z

  lxc exec micro04 -- tail -1 out | grep "Failed waiting during join: Initiator aborted the setup" -q

+  # Initiate a MicroCloud cluster with a short session timeout and verify the joiner receives "Session timeout exceeded".
+  reset_systems 2 3 0


2 is the number of peers, 3 number of disks, 0 number of extra interfaces.

As we never reach the config stage and don't require a disk, you can also do 2 0 0 here.

Copilot

Pull request overview

This PR improves MicroCloud’s WebSocket session handling so that when a trust establishment session expires, the client receives the real cancellation cause (e.g., “Session timeout exceeded”) rather than a generic abnormal WebSocket EOF error, aligning behavior with issue #1245.

Changes:

Adjusts the WebSocket gateway close-path to reliably send a ControlClose message even when the gateway context is canceled.
Improves receive behavior to surface the gateway context cause when the receive channel closes.
Adds a system test intended to validate that both initiator and joiner surface the timeout error.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 3 comments.

File	Description
client/websocket.go	Ensures close-cause forwarding works reliably and improves receive-side error reporting when the socket closes.
test/suites/basic.sh	Adds an interactive test case intended to validate session-timeout error propagation to initiator/joiner.

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

+  export SESSION_TIMEOUT="5"
+
+  ! join_session init micro01 micro02 || false


+  # Initiate a MicroCloud cluster with a short session timeout and verify the joiner receives "Session timeout exceeded".
+  reset_systems 2 3 0
+  unset_interactive_vars
+
+  echo "Initiate a MicroCloud cluster with a short session timeout and verify the joiner receives the timeout error"


+				return context.Cause(w.ctx)
+			}
+
+			return errors.New("Websocket connection closed")


shardool-patil · 2026-08-21T11:24:35Z

@roosterfish i have addressed all the requested changes:

test/suites/basic.sh: Set EXPECT_PEERS=2, clarified the test comment and echo message to verify both initiator and joiner, and added the missing EOF newline[cite: 2, 3].
test/includes/microcloud.sh: Updated microcloud_interactive to pass ${SESSION_TIMEOUT:-60} dynamically and added SESSION_TIMEOUT to unset_interactive_vars[cite: 2].

take a look whenever possible

Signed-off-by: Shardool Patil <shardoolpatil999@gmail.com>

roosterfish · 2026-08-21T13:35:57Z

See the test failure https://github.com/canonical/microcloud/actions/runs/32477357735/job/96766897819?pr=1519#step:3:45500.
I suspect what is happening is that the session expires too early now. Maybe let's leave the 60s default (no need to make it configurable in this case).

shardool-patil · 2026-08-21T16:43:09Z

See the test failure https://github.com/canonical/microcloud/actions/runs/32477357735/job/96766897819?pr=1519#step:3:45500. I suspect what is happening is that the session expires too early now. Maybe let's leave the 60s default (no need to make it configurable in this case).

@roosterfish I have worked and handled the requested changes so take a look at the code.

shardool-patil · 2026-08-25T02:27:42Z

@roosterfish I switched the test to reset_systems 3 0 0 because with reset_systems 2 0 0, micro03 is paused by default. Since EXPECT_PEERS=2 triggers table:expect 2, micro01 hangs during mDNS peer discovery waiting for a nonexistent second peer before the session ever starts. Running reset_systems 3 0 0 allows micro01 to discover both peers, select them, connect with micro02, and wait for micro03 until the session timeout triggers. Does reset_systems 3 0 0 work for you here?

Signed-off-by: Shardool Patil <shardoolpatil999@gmail.com>

shardool-patil · 2026-08-25T16:19:32Z

@roosterfish I've updated test/suites/basic.sh:

Switched to reset_systems 3 0 0 and disabled storage setup (SETUP_ZFS="no", SETUP_CEPH="no").
Kept EXPECT_PEERS=2 with join_session init micro01 micro02 so micro01 connects with micro02 and waits for micro03 until the default 60s session timeout triggers.
With reset_systems 2 0 0, micro03 is paused by default, causing micro01 to hang during mDNS peer discovery (table:expect 2) before the session can start. 3 0 0 keeps micro03 running so discovery succeeds without configuring any unused storage devices.

Please take a look at this setup before re-running the tests.

roosterfish requested changes Aug 21, 2026

View reviewed changes

roosterfish requested a lite review from Copilot August 21, 2026 10:11

Copilot started reviewing on behalf of roosterfish August 21, 2026 10:11 View session

Copilot AI reviewed Aug 21, 2026

View reviewed changes

client: forward session timeout cause over websocket

8ef03e7

Signed-off-by: Shardool Patil <shardoolpatil999@gmail.com>

shardool-patil force-pushed the fix/session-timeout-forwarding branch from b287951 to 4ffe082 Compare August 21, 2026 11:28

roosterfish marked this pull request as draft August 21, 2026 15:41

shardool-patil force-pushed the fix/session-timeout-forwarding branch from 4ffe082 to 14a4c0b Compare August 21, 2026 16:41

shardool-patil requested a review from roosterfish August 21, 2026 17:35

roosterfish marked this pull request as ready for review August 24, 2026 11:42

shardool-patil force-pushed the fix/session-timeout-forwarding branch from 14a4c0b to c3f95c6 Compare August 25, 2026 02:27

test/suites: add session timeout test to basic suite

e98fe8e

Signed-off-by: Shardool Patil <shardoolpatil999@gmail.com>

shardool-patil force-pushed the fix/session-timeout-forwarding branch from c3f95c6 to e98fe8e Compare August 25, 2026 16:16

Navigation Menu

Uh oh!

client, test/suites: forward session timeout cause over websocket - #1519

client, test/suites: forward session timeout cause over websocket#1519
shardool-patil wants to merge 2 commits into
canonical:mainfrom
shardool-patil:fix/session-timeout-forwarding

shardool-patil commented Aug 21, 2026

Uh oh!

roosterfish left a comment

Uh oh!

roosterfish Aug 21, 2026

Uh oh!

roosterfish Aug 21, 2026

Uh oh!

roosterfish Aug 21, 2026

Uh oh!

Copilot AI left a comment

Uh oh!

shardool-patil commented Aug 21, 2026

Uh oh!

roosterfish commented Aug 21, 2026

Uh oh!

shardool-patil commented Aug 21, 2026

Uh oh!

shardool-patil commented Aug 25, 2026

Uh oh!

shardool-patil commented Aug 25, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

		export SESSION_TIMEOUT="5"

		! join_session init micro01 micro02 \|\| false

Uh oh!

Conversation

shardool-patil commented Aug 21, 2026

Description

Changes

Checklist

Related issues

Uh oh!

roosterfish left a comment

Choose a reason for hiding this comment

Uh oh!

roosterfish Aug 21, 2026

Choose a reason for hiding this comment

Uh oh!

roosterfish Aug 21, 2026

Choose a reason for hiding this comment

Uh oh!

roosterfish Aug 21, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

shardool-patil commented Aug 21, 2026

Uh oh!

roosterfish commented Aug 21, 2026

Uh oh!

shardool-patil commented Aug 21, 2026

Uh oh!

shardool-patil commented Aug 25, 2026

Uh oh!

shardool-patil commented Aug 25, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants