[grid] Accept legacy session-closed event payloads

[VietND96]

🔗 Related Issues

Fixes SeleniumHQ/docker-selenium#3110

💥 What does this PR do?

Backward compatibility for Grid session-closed events when the event bus receives the legacy wire format: a bare JSON string session id instead of a SessionClosedData object.

In 4.41.0, listeners in the distributor and session map deserialize session-closed as SessionClosedData. When an older or custom publisher emits the legacy payload, deserialization fails with Expected to read a START_MAP but instead have: STRING, which prevents the listener from processing the event.

🔧 Implementation Notes

💡 Additional Considerations

🔄 Types of changes

• Cleanup (formatting, renaming)
• Bug fix (backwards compatible)
• New feature (non-breaking change which adds functionality and tests!)
• Breaking change (fix or feature that would cause existing functionality to change)

[qodo-code-review]

Review Summary by Qodo

Accept legacy session-closed event payloads for backward compatibility

🐞 Bug fix

Walkthroughs

Description

• Accept legacy session-closed event payloads with bare SessionId strings
• Deserialize legacy wire format to SessionClosedData with QUIT_COMMAND reason
• Add comprehensive tests for backward compatibility handling

File Changes

1. java/src/org/openqa/selenium/grid/data/SessionClosedData.java 🐞 Bug fix +5/-0

Add legacy session-closed payload deserialization support

• Import JsonType for type checking during deserialization
• Add legacy payload handling in fromJson() method to detect STRING type
• Deserialize bare SessionId strings as SessionClosedData with QUIT_COMMAND reason
• Maintain backward compatibility with older or custom event publishers

java/src/org/openqa/selenium/grid/data/SessionClosedData.java

---

2. java/test/org/openqa/selenium/grid/data/SessionClosedDataTest.java 🧪 Tests +64/-0

Add tests for legacy session-closed payload handling

• Create new test class for SessionClosedData backward compatibility
• Test deserialization of legacy SessionId string payload format
• Verify event listeners correctly handle legacy session-closed events
• Validate SessionClosedReason defaults to QUIT_COMMAND for legacy payloads

java/test/org/openqa/selenium/grid/data/SessionClosedDataTest.java

---

3. java/test/org/openqa/selenium/grid/data/BUILD.bazel ⚙️ Configuration changes +1/-0

Add events dependency for test suite

• Add dependency on //java/src/org/openqa/selenium/events for test support
• Enable SessionClosedEvent listener testing in SessionClosedDataTest

java/test/org/openqa/selenium/grid/data/BUILD.bazel

---

[qodo-code-review]

Code Review by Qodo

🐞 Bugs (0)   📘 Rule violations (1)   📎 Requirement gaps (0)

📘\ ☼ Reliability (1)

1. Legacy sessionId not validated 📘 ☼

Description

The new legacy JSON STRING payload handling accepts any session id string without validating it is
non-empty/usable, which can allow malformed protocol-derived input to flow deeper and fail later
with less actionable errors.

Code

java/src/org/openqa/selenium/grid/data/SessionClosedData.java[R144-146]

+    if (input.peek() == JsonType.STRING) {
+      return new SessionClosedData(input.read(SessionId.class), SessionClosedReason.QUIT_COMMAND);
+    }

Evidence

PR Compliance ID 15 requires early validation of external/protocol-derived inputs (e.g.,
non-null/non-empty) with clear exceptions. The added legacy branch reads a SessionId from a JSON
string and immediately constructs SessionClosedData without validating the value;
SessionId(String) only enforces non-null, so empty strings can pass through silently.

java/src/org/openqa/selenium/grid/data/SessionClosedData.java[144-146]
java/src/org/openqa/selenium/remote/SessionId.java[37-39]
Best Practice: Learned patterns

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
The legacy `STRING` payload deserialization path for `SessionClosedData` does not validate the protocol-derived session id value (e.g., non-empty), which can let malformed values through and produce harder-to-debug downstream failures.

## Issue Context
The legacy compatibility branch triggers when `input.peek() == JsonType.STRING` and then calls `input.read(SessionId.class)`; `SessionId(String)` only checks for non-null.

## Fix Focus Areas
- java/src/org/openqa/selenium/grid/data/SessionClosedData.java[144-146]
- java/src/org/openqa/selenium/remote/SessionId.java[37-39]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

---

ⓘ The new review experience is currently in Beta. Learn more