Skip to content

Docker: Retain recordings for failed sessions only from session capabilities #3111

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

Merged
VietND96 merged 2 commits into from
Apr 12, 2026
Merged

Docker: Retain recordings for failed sessions only from session capabilities #3111

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
a5fee57
Docker: Retain recordings for failed sessions only from session capab…
VietND96 Apr 10, 2026
ccdefeb
Merge branch 'trunk' into retain-on-failure
VietND96 Apr 12, 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
15 changes: 15 additions & 0 deletions .github/workflows/docker-test.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -55,34 +55,39 @@ jobs:
os: ubuntu-24.04
firefox-install-lang-package:
enable-managed-downloads:
retain-on-failure:
- test-strategy: test_video_dynamic_name
use-random-user: false
test-video: true
build-all: false
os: ubuntu-24.04
firefox-install-lang-package:
enable-managed-downloads:
retain-on-failure:
- test-strategy: test_video_standalone
use-random-user: false
test-video: true
build-all: false
os: ubuntu-24.04
firefox-install-lang-package:
enable-managed-downloads:
retain-on-failure:
- test-strategy: test_node_docker
use-random-user: false
test-video: true
build-all: false
os: ubuntu-24.04
firefox-install-lang-package:
enable-managed-downloads:
retain-on-failure:
- test-strategy: test_standalone_docker
use-random-user: false
test-video: true
build-all: false
os: ubuntu-24.04
firefox-install-lang-package:
enable-managed-downloads:
retain-on-failure:
- test-strategy: test_parallel
use-random-user: false
test-video: false
Expand Down Expand Up @@ -119,34 +124,39 @@ jobs:
os: ubuntu-24.04-arm
firefox-install-lang-package: true
enable-managed-downloads: true
retain-on-failure: true
- test-strategy: test_video_dynamic_name
use-random-user: false
test-video: true
build-all: false
os: ubuntu-24.04-arm
firefox-install-lang-package: true
enable-managed-downloads: true
retain-on-failure: true
- test-strategy: test_video_standalone
use-random-user: false
test-video: true
build-all: false
os: ubuntu-24.04-arm
firefox-install-lang-package: true
enable-managed-downloads: true
retain-on-failure: true
- test-strategy: test_node_docker_video_sidecar
use-random-user: false
test-video: true
build-all: false
os: ubuntu-24.04-arm
firefox-install-lang-package: true
enable-managed-downloads: false
retain-on-failure: true
- test-strategy: test_standalone_docker_video_sidecar
use-random-user: false
test-video: true
build-all: false
os: ubuntu-24.04-arm
firefox-install-lang-package: true
enable-managed-downloads: true
retain-on-failure: true
- test-strategy: test_parallel
use-random-user: false
test-video: false
Expand Down Expand Up @@ -246,9 +256,13 @@ jobs:
if [ -n "${SELENIUM_ENABLE_MANAGED_DOWNLOADS}" ]; then
echo "SELENIUM_ENABLE_MANAGED_DOWNLOADS=${SELENIUM_ENABLE_MANAGED_DOWNLOADS}" >> $GITHUB_ENV
fi
if [ -n "${TEST_RETAIN_ON_FAILURE}" ]; then
echo "TEST_RETAIN_ON_FAILURE=${TEST_RETAIN_ON_FAILURE}" >> $GITHUB_ENV
Comment on lines +259 to +260
Copy link
Copy Markdown
Contributor

@qodo-code-review qodo-code-review bot Apr 10, 2026

Choose a reason for hiding this comment

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

Action required

1. Unquoted $github_env redirection 📘 Rule violation ☼ Reliability

The new workflow line appends to $GITHUB_ENV without quoting the path, which is brittle and can
break if the path contains spaces or special characters. This violates the requirement to quote
variables/paths in shell scripts to avoid word-splitting/globbing issues.
Agent Prompt 
## Issue description
The workflow appends to `GITHUB_ENV` using an unquoted path (`>> $GITHUB_ENV`), which is brittle under shell word-splitting rules.

## Issue Context
GitHub Actions recommends treating `GITHUB_ENV` as a file path; quoting prevents failures if the path ever contains spaces/special characters.

## Fix Focus Areas
- .github/workflows/docker-test.yml[259-261]

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

fi
env:
TEST_FIREFOX_INSTALL_LANG_PACKAGE: ${{ matrix.firefox-install-lang-package }}
SELENIUM_ENABLE_MANAGED_DOWNLOADS: ${{ matrix.enable-managed-downloads }}
TEST_RETAIN_ON_FAILURE: ${{ matrix.retain-on-failure }}
- name: Run Docker Compose to ${{ matrix.test-strategy }} on AMD64
if: contains(matrix.os, 'arm') == false
uses: nick-invision/retry@master
Expand All @@ -268,6 +282,7 @@ jobs:
command: |
USE_RANDOM_USER_ID=${{ matrix.use-random-user }} VERSION=${BRANCH} BUILD_DATE=${BUILD_DATE} \
TEST_FIREFOX_INSTALL_LANG_PACKAGE=${TEST_FIREFOX_INSTALL_LANG_PACKAGE} SELENIUM_ENABLE_MANAGED_DOWNLOADS=${SELENIUM_ENABLE_MANAGED_DOWNLOADS} \
TEST_RETAIN_ON_FAILURE=${TEST_RETAIN_ON_FAILURE} \
make ${{ matrix.test-strategy }}
- name: Upload recorded video
if: matrix.test-video == true
Expand Down
8 changes: 4 additions & 4 deletions ENV_VARIABLES.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,11 +2,11 @@
|--------------|---------------|-------------|----------------------|
| SE_SCREEN_WIDTH | 1920 | Use in Node to set the screen width | |
| SE_SCREEN_HEIGHT | 1080 | Use in Node to set the screen height | |
| SE_VIDEO_FILE_NAME | auto | Use in function video recording to set the output file name. Set `auto` for dynamic file name relying on test metadata | |
| SE_VIDEO_FILE_NAME | video.mp4 | Use in function video recording to set the output file name. Set `auto` for dynamic file name relying on test metadata | |
| SE_FRAME_RATE | 15 | Set the frame rate for FFmpeg in video recording | |
| SE_CODEC | libx264 | Set the codec for FFmpeg in video recording | |
| SE_PRESET | -preset ultrafast | Set the preset for FFmpeg in video recording | |
| SE_VIDEO_UPLOAD_ENABLED | false | Enable video upload | |
| SE_VIDEO_UPLOAD_ENABLED | false | Deprecated in event-driven mode (`video_service.py`); upload enablement is now derived from non-empty `SE_UPLOAD_DESTINATION_PREFIX` | |
| SE_VIDEO_INTERNAL_UPLOAD | true | Enable video upload using Rclone in the same recorder container | |
| SE_UPLOAD_DESTINATION_PREFIX | | Remote name and destination path to upload | |
| SE_UPLOAD_PIPE_FILE_NAME | | Set the pipe file name for video upload to consume | |
Expand Down Expand Up @@ -162,9 +162,9 @@
| SE_BIND_BUS | true | When true, the Standalone will start the Event Bus and connect itself. Standalone also expose publishing and subscribing port for sidecar service can listen on session events. | --bind-bus |
| SE_EVENT_BUS_IMPLEMENTATION | | Full class name of non-default event bus implementation. For example: org.openqa.selenium.events.zeromq.ZeroMqEventBus | --events-implementation |
| SE_NODE_KUBERNETES_CONFIG_FILENAME | kubernetes.toml | A separate TOML config file name for Dynamic Grid in Kubernetes, which avoid conflict with browser config if shared mouted volume | |
| SE_UPLOAD_FAILURE_SESSION_EVENTS | :failed,:failure | By default, a failure session event type contains ":failed" or ":failure" fires that will trigger the upload failure session only. User can define more event types which handled in your test framework, separated by comma. | |
| SE_UPLOAD_FAILURE_SESSION_ONLY | false | When true, only recording of sessions that are not exited normally (session timed out, or custom events were fired by the client match with failure events defined) | |
| SE_VIDEO_EVENT_DRIVEN | true | Backend of video recorder and uploader will subscribe to Grid Event Bus for session event lifecycle for processing instead of traditional polling Node session capabilities via /status endpoint. | |
| SE_PLAIN_LOGS | true | Use plain log lines | --plain-logs |
| SE_DYNAMIC_MAX_SESSIONS | | Set the number of maximum concurrent sessions of Dynamic Node (both Docker and Kubernetes) | |
| SE_DYNAMIC_OVERRIDE_MAX_SESSIONS | | Enable this flag for setting max session take effect in Dynamic Node (both Docker and Kubernetes) | |
| SE_FAILURE_SESSION_EVENTS | :failed,:failure,:error,:aborted | By default, a failure session event type contains ":failed", ":failure", ":error" or ":aborted" substrings that trigger the retain-on-failure sub-sequence. User can define more event types which handled in your test framework, separated by comma. | |
| SE_RETAIN_ON_FAILURE | false | When true, recordings for sessions that pass are discarded immediately. Only sessions that fail (via failure events or abnormal close) retain their recordings and are queued for upload. | |
2 changes: 2 additions & 0 deletions Makefile
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1111,6 +1111,7 @@ test_video: video hub chrome firefox edge chromium
echo BASIC_AUTH_PASSWORD=$(or $(BASIC_AUTH_PASSWORD), "admin") >> .env ; \
echo SUB_PATH=$(or $(SUB_PATH), "/selenium") >> .env ; \
echo TEST_ADD_CAPS_RECORD_VIDEO=$(or $(TEST_ADD_CAPS_RECORD_VIDEO), "true") >> .env ; \
echo TEST_RETAIN_ON_FAILURE=$(or $(TEST_RETAIN_ON_FAILURE), "false") >> .env ; \
if [ $$node = "NodeChrome" ] ; then \
echo BROWSER=chrome >> .env ; \
echo VIDEO_FILE_NAME=$${VIDEO_FILE_NAME:-"chrome_video.mp4"} >> .env ; \
Expand Down Expand Up @@ -1230,6 +1231,7 @@ test_node_docker: hub standalone_docker standalone_chrome standalone_firefox sta
echo GRID_URL=$(or $(GRID_URL), "") >> .env ; \
echo HUB_CHECKS_INTERVAL=$(or $(HUB_CHECKS_INTERVAL), 20) >> .env ; \
echo TEST_CUSTOM_SPECIFIC_NAME=$(or $(TEST_CUSTOM_SPECIFIC_NAME), "true") >> .env ; \
echo TEST_RETAIN_ON_FAILURE=$(or $(TEST_RETAIN_ON_FAILURE), "false") >> .env ; \
echo NODE=$$node >> .env ; \
echo UID=$$(id -u) >> .env ; \
echo BINDING_VERSION=$(BINDING_VERSION) >> .env ; \
Expand Down
66 changes: 65 additions & 1 deletion README.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -813,7 +813,7 @@ services:
`SE_VIDEO_FILE_NAME=auto` will use the session id as the video file name. This ensures that the video file name is unique to upload.
Video file name construction automatically works based on Node endpoint `/status` (and optional GraphQL endpoint) to get session ID, capabilities.

`SE_VIDEO_UPLOAD_ENABLED=true` (`false` by default) will enable the video upload feature. In the background, it will create a pipefile with file and destination for uploader to consume and proceed.
`SE_VIDEO_UPLOAD_ENABLED=true` enables upload in the legacy shell-based mode (`SE_VIDEO_EVENT_DRIVEN=false`). In event-driven mode (the default), this variable is **deprecated** — upload is enabled automatically when `SE_UPLOAD_DESTINATION_PREFIX` is set to a non-empty value.

`SE_VIDEO_INTERNAL_UPLOAD=true` (by default) will use RCLONE installed in the container for upload. If you want to use another sidecar container for upload, set it to `false`.

Expand Down Expand Up @@ -846,6 +846,70 @@ When using in Dynamic Grid, those variables should be combined with the prefix `
| `SE_UPLOAD_CONFIG_FILE_NAME` | `upload.conf` | Config file for remote host instead of set via env variable prefix SE_RCLONE_* |
| `SE_UPLOAD_CONFIG_DIRECTORY` | `/opt/bin` | Directory of config file (change it when conf file in another directory is mounted) |

### Retain recordings for failed sessions only

In event-driven mode (`SE_VIDEO_EVENT_DRIVEN=true`, the default), the video service subscribes to the Grid's ZeroMQ event bus and reacts to session lifecycle events in real time. This enables a **retain-on-failure** strategy: record every session, but automatically discard the video when the session passes and only keep (and upload) recordings from sessions that fail.

Enable it globally with the environment variable:

```yaml
SE_RETAIN_ON_FAILURE=true
```

A session is treated as **failed** when either of the following is true:

1. The test code fires a session event whose `eventType` contains a substring from `SE_FAILURE_SESSION_EVENTS` (default: `:failed,:failure,:error,:aborted`).
2. The session closes with an abnormal reason — `TIMEOUT`, `NODE_REMOVED`, or `NODE_RESTARTED` — instead of the normal `QUIT_COMMAND`.

| Environment variable | Default | Description |
|---------------------------|----------------------------------|-------------------------------------------------------------------------------------------------------------------------------------|
| `SE_RETAIN_ON_FAILURE` | `false` | Discard recordings of sessions that pass. Only recordings from failed sessions are retained on disk and queued for upload. |
| `SE_FAILURE_SESSION_EVENTS` | `:failed,:failure,:error,:aborted` | Comma-separated substrings. Any session event whose `eventType` contains one of these (case-insensitive) marks the session as failed. |

The `se:retainOnFailure` session **capability** overrides the global container env var for a specific session. For example, to retain the recording of a single session regardless of the global setting:

```python
options.set_capability('se:retainOnFailure', True)
```

| `se:retainOnFailure` cap | `SE_RETAIN_ON_FAILURE` env | Effective behaviour |
|--------------------------|----------------------------|----------------------------------|
| `true` | `false` (default) | Retain on failure for this session |
| `false` | `true` | Always retain for this session |
| absent | `true` | Retain on failure (global default) |
| absent | `false` (default) | Always retain (global default) |

#### Firing session events from test code

The [Session Event API](https://www.selenium.dev/blog/2026/selenium-grid-4-41-deep-dive/) lets test code push named events directly to the Grid. The video service listens for these events on the ZeroMQ bus and uses them to determine session failure.

Call `driver.fire_session_event(eventType, payload)` from your test. Any `eventType` that contains a configured failure substring (e.g. `"test:failed"` contains `":failed"`) marks the session as failed.

```python
from selenium.webdriver.chrome.options import Options as ChromeOptions
from selenium import webdriver

options = ChromeOptions()
options.set_capability('se:name', 'checkout_flow')
options.set_capability('se:retainOnFailure', True) # discard video if this session passes

driver = webdriver.Remote(options=options, command_executor="http://localhost:4444")

try:
driver.get("https://selenium.dev")
# ... test steps ...
except Exception as exc:
# "test:failed" contains ":failed" — matches the default SE_FAILURE_SESSION_EVENTS
driver.fire_session_event("test:failed", {"error": str(exc)})
raise
finally:
driver.quit()
```

> **Note:** If the test catches an exception and still calls `driver.quit()` normally, the session close reason is `QUIT_COMMAND` (not abnormal). In that case, firing a failure event **before** `quit()` is the only way to mark the session as failed and prevent the recording from being discarded.

So, you can control the **retain-on-failure** strategy fully from test code via session capabilities and fire session event.

## Video recordings manager

We utilize [File Browser](https://filebrowser.org/) as a video manager. It is a web-based file manager that allows you to manage files and folders in the storage.
Expand Down
4 changes: 2 additions & 2 deletions Video/Dockerfile
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -52,7 +52,7 @@ ENV DISPLAY_NUM="99" \
SE_VIDEO_UPLOAD_ENABLED="false" \
SE_VIDEO_INTERNAL_UPLOAD="true" \
SE_UPLOAD_DESTINATION_PREFIX="" \
SE_UPLOAD_FAILURE_SESSION_ONLY="false" \
SE_UPLOAD_FAILURE_SESSION_EVENTS=":failed,:failure"
SE_RETAIN_ON_FAILURE="false" \
SE_FAILURE_SESSION_EVENTS=":failed,:failure,:error,:aborted"

EXPOSE 9000
24 changes: 18 additions & 6 deletions Video/video_ready.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,13 +12,25 @@
class Handler(BaseHTTPRequestHandler):

def do_GET(self):
if (
environ.get('SE_VIDEO_UPLOAD_ENABLED', 'false').lower() != 'true'
and environ.get('SE_VIDEO_FILE_NAME', 'video.mp4').lower() != 'auto'
):
video_ready = "ffmpeg" in (p.name().lower() for p in psutil.process_iter())
else:
event_driven = environ.get('SE_VIDEO_EVENT_DRIVEN', 'false').lower() == 'true'
record_video_enabled = environ.get('SE_RECORD_VIDEO', 'true').lower() == 'true'

# Legacy shell mode compatibility:
# when global recording is disabled, report ready immediately.
if not event_driven and not record_video_enabled:
video_ready = True
else:
# Event-driven mode enables upload by non-empty destination.
# Legacy shell mode keeps original SE_VIDEO_UPLOAD_ENABLED behaviour.
if event_driven:
upload_enabled = bool(environ.get('SE_UPLOAD_DESTINATION_PREFIX', '').strip())
else:
upload_enabled = environ.get('SE_VIDEO_UPLOAD_ENABLED', 'false').lower() == 'true'

if not upload_enabled and environ.get('SE_VIDEO_FILE_NAME', 'video.mp4').lower() != 'auto':
video_ready = "ffmpeg" in (p.name().lower() for p in psutil.process_iter())
else:
video_ready = True
response_code = 200 if video_ready else 404
response_text = "ready" if video_ready else "not ready"
self.send_response(response_code)
Expand Down
Loading
Loading