Skip to content

fix: collect adb errors when launching MjpegServer or AvcServer #168

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

Open
gmegidish wants to merge 1 commit into
base: main
Choose a base branch
from
Open

fix: collect adb errors when launching MjpegServer or AvcServer #168

Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
135 changes: 119 additions & 16 deletions devices/android.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ import (
"encoding/base64"
"encoding/xml"
"fmt"
"io"
"os"
"os/exec"
"path/filepath"
Expand Down Expand Up @@ -875,6 +876,103 @@ func (d *AndroidDevice) GetAppPath(packageName string) (string, error) {
return appPath, nil
}

const (
stderrBufferSize = 4096
stdoutBufferSize = 65536
serverStartupTimeout = 500 * time.Millisecond
)

func containsServerError(output string) bool {
errorPatterns := []string{
"ClassNotFoundException",
"NoSuchMethodException",
"Error:",
"Fatal signal",
"SIGABRT",
}

for _, pattern := range errorPatterns {
if strings.Contains(output, pattern) {
return true
}
}
return false
}

func (d *AndroidDevice) parseServerError(msg, serverClass string) error {
if strings.Contains(msg, "ClassNotFoundException") {
return fmt.Errorf("server class not found: %s. the installed APK may be outdated or corrupted. try reinstalling", serverClass)
}
if strings.Contains(msg, "NoSuchMethodException") {
return fmt.Errorf("incompatible Android version or corrupted APK. the server failed to create virtual display")
}
if strings.Contains(msg, "Fatal signal") || strings.Contains(msg, "SIGABRT") {
return fmt.Errorf("server crashed during startup: %s", msg)
}
return fmt.Errorf("server error: %s", msg)
}

func (d *AndroidDevice) monitorServerStartup(stderr io.Reader, serverClass string, errorChan chan<- error) {
buffer := make([]byte, stderrBufferSize)

for {
n, err := stderr.Read(buffer)
if n > 0 {
msg := string(buffer[:n])
utils.Verbose("stderr: %s", msg)

if containsServerError(msg) {
errorChan <- d.parseServerError(msg, serverClass)
return
}
}
if err != nil {
return
}
}
}
Comment on lines +915 to +933
Copy link

@coderabbitai coderabbitai bot Feb 9, 2026

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟡 Minor

Stderr error detection can miss errors split across read boundaries.

stderr.Read(buffer) may return partial data. If an error string like "ClassNotFoundException" is split across two reads, containsServerError won't match it on either chunk. Consider accumulating stderr into a bytes.Buffer (or using bufio.Scanner with line-based reading) so pattern matching operates on the full output.

Proposed fix using accumulated buffer 
 func (d *AndroidDevice) monitorServerStartup(stderr io.Reader, serverClass string, errorChan chan<- error) {
-	buffer := make([]byte, stderrBufferSize)
+	var accumulated strings.Builder
+	readBuf := make([]byte, stderrBufferSize)
 
 	for {
-		n, err := stderr.Read(buffer)
+		n, err := stderr.Read(readBuf)
 		if n > 0 {
-			msg := string(buffer[:n])
+			accumulated.Write(readBuf[:n])
+			msg := accumulated.String()
 			utils.Verbose("stderr: %s", msg)
 
 			if containsServerError(msg) {
 				errorChan <- d.parseServerError(msg, serverClass)
 				return
 			}
 		}
 		if err != nil {
 			return
 		}
 	}
 }
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
func (d *AndroidDevice) monitorServerStartup(stderr io.Reader, serverClass string, errorChan chan<- error) {
buffer := make([]byte, stderrBufferSize)
for {
n, err := stderr.Read(buffer)
if n > 0 {
msg := string(buffer[:n])
utils.Verbose("stderr: %s", msg)
if containsServerError(msg) {
errorChan <- d.parseServerError(msg, serverClass)
return
}
}
if err != nil {
return
}
}
}
func (d *AndroidDevice) monitorServerStartup(stderr io.Reader, serverClass string, errorChan chan<- error) {
var accumulated strings.Builder
readBuf := make([]byte, stderrBufferSize)
for {
n, err := stderr.Read(readBuf)
if n > 0 {
accumulated.Write(readBuf[:n])
msg := accumulated.String()
utils.Verbose("stderr: %s", msg)
if containsServerError(msg) {
errorChan <- d.parseServerError(msg, serverClass)
return
}
}
if err != nil {
return
}
}
}
🤖 Prompt for AI Agents 
In `@devices/android.go` around lines 915 - 933, In monitorServerStartup, stderr
is read in chunks so containsServerError can miss tokens split across reads;
change the loop to accumulate reads into a growing buffer (e.g., bytes.Buffer or
a bufio.Reader/Scanner that emits full lines) and run containsServerError
against the accumulated data (or each complete line) so multi-chunk patterns
like "ClassNotFoundException" are detected; keep using parseServerError and
errorChan for reporting, and add a reasonable cap or truncation to the buffer
(to avoid unbounded growth) using the existing stderrBufferSize as a guide.


func (d *AndroidDevice) waitForServerStartup(errorChan <-chan error, cmd *exec.Cmd) error {
time.Sleep(serverStartupTimeout)
select {
case startupErr := <-errorChan:
_ = cmd.Process.Kill()
return startupErr
default:
return nil
}
}

func (d *AndroidDevice) streamServerOutput(stdout io.Reader, cmd *exec.Cmd, config ScreenCaptureConfig) error {
buffer := make([]byte, stdoutBufferSize)
firstRead := true

for {
n, err := stdout.Read(buffer)
if err != nil {
break
}

if n > 0 {
if firstRead {
firstRead = false
output := string(buffer[:n])
if containsServerError(output) {
_ = cmd.Process.Kill()
return fmt.Errorf("server error: %s", strings.TrimSpace(output))
}
}

if !config.OnData(buffer[:n]) {
break
}
}
}

_ = cmd.Process.Kill()
return nil
}
Comment on lines +946 to +974
Copy link

@coderabbitai coderabbitai bot Feb 9, 2026

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major

cmd.Wait() is never called — process resources will leak.

After cmd.Process.Kill() (or when the process exits naturally), cmd.Wait() must be called to release associated OS resources (file descriptors, process table entry). Without it, the process becomes a zombie. This applies to both the normal exit path and the error path in waitForServerStartup.

Proposed fix 
 func (d *AndroidDevice) streamServerOutput(stdout io.Reader, cmd *exec.Cmd, config ScreenCaptureConfig) error {
 	buffer := make([]byte, stdoutBufferSize)
 	firstRead := true
 
 	for {
 		n, err := stdout.Read(buffer)
 		if err != nil {
 			break
 		}
 
 		if n > 0 {
 			if firstRead {
 				firstRead = false
 				output := string(buffer[:n])
 				if containsServerError(output) {
 					_ = cmd.Process.Kill()
+					_ = cmd.Wait()
 					return fmt.Errorf("server error: %s", strings.TrimSpace(output))
 				}
 			}
 
 			if !config.OnData(buffer[:n]) {
 				break
 			}
 		}
 	}
 
 	_ = cmd.Process.Kill()
+	_ = cmd.Wait()
 	return nil
 }

Also add _ = cmd.Wait() in waitForServerStartup after the Kill:

 func (d *AndroidDevice) waitForServerStartup(errorChan <-chan error, cmd *exec.Cmd) error {
 	time.Sleep(serverStartupTimeout)
 	select {
 	case startupErr := <-errorChan:
 		_ = cmd.Process.Kill()
+		_ = cmd.Wait()
 		return startupErr
 	default:
 		return nil
 	}
 }
🤖 Prompt for AI Agents 
In `@devices/android.go` around lines 946 - 974, The streamServerOutput function
(and similarly waitForServerStartup) currently kills the process but never calls
cmd.Wait(), leaking OS resources; update streamServerOutput to call cmd.Wait()
after any call to cmd.Process.Kill() and also call cmd.Wait() on the normal exit
path (after the read loop) to reap the child, and ensure waitForServerStartup
likewise calls cmd.Wait() after killing the process or when the process exits;
ignore or log the returned error from cmd.Wait() as appropriate but always
invoke it to prevent zombies.


func (d *AndroidDevice) StartScreenCapture(config ScreenCaptureConfig) error {
if config.Format != "mjpeg" && config.Format != "avc" {
return fmt.Errorf("unsupported format: %s, only 'mjpeg' and 'avc' are supported", config.Format)
Expand Down Expand Up @@ -907,7 +1005,17 @@ func (d *AndroidDevice) StartScreenCapture(config ScreenCaptureConfig) error {
}

utils.Verbose("Starting %s with app path: %s", serverClass, appPath)
cmdArgs := append([]string{"-s", d.getAdbIdentifier()}, "exec-out", fmt.Sprintf("CLASSPATH=%s", appPath), "app_process", "/system/bin", serverClass, "--quality", fmt.Sprintf("%d", config.Quality), "--scale", fmt.Sprintf("%.2f", config.Scale), "--fps", fmt.Sprintf("%d", config.FPS))
cmdArgs := append(
[]string{"-s", d.getAdbIdentifier()},
"exec-out",
fmt.Sprintf("CLASSPATH=%s", appPath),
"app_process",
"/system/bin",
serverClass,
"--quality", fmt.Sprintf("%d", config.Quality),
"--scale", fmt.Sprintf("%.2f", config.Scale),
"--fps", fmt.Sprintf("%d", config.FPS),
)
utils.Verbose("Running command: %s %s", getAdbPath(), strings.Join(cmdArgs, " "))
cmd := exec.Command(getAdbPath(), cmdArgs...)

Expand All @@ -916,28 +1024,23 @@ func (d *AndroidDevice) StartScreenCapture(config ScreenCaptureConfig) error {
return fmt.Errorf("failed to create stdout pipe: %v", err)
}

stderr, err := cmd.StderrPipe()
if err != nil {
return fmt.Errorf("failed to create stderr pipe: %v", err)
}

if err := cmd.Start(); err != nil {
return fmt.Errorf("failed to start %s: %v", serverClass, err)
}

// Read bytes from the command output and send to callback
buffer := make([]byte, 65536)
for {
n, err := stdout.Read(buffer)
if err != nil {
break
}
startupErrorChan := make(chan error, 1)
go d.monitorServerStartup(stderr, serverClass, startupErrorChan)

if n > 0 {
// Send bytes to callback, break if it returns false
if !config.OnData(buffer[:n]) {
break
}
}
if err := d.waitForServerStartup(startupErrorChan, cmd); err != nil {
return err
}

_ = cmd.Process.Kill()
return nil
return d.streamServerOutput(stdout, cmd, config)
}

func (d *AndroidDevice) installPackage(apkPath string) error {
Expand Down
Loading