[v2] Improvements to the dev command (#1510)

* [v2, darwin] Fix nullreference exception when dev watcher failed to start and the user stopped wails dev with ctrl-c

* [v2] Improve starting of dev watcher command

Wails dev now also fails if the dev watcher could not be started. Since
the dev watcher is used to start e.g. vite to serve the frontend, it does
not make much sense to let Wails dev be running if the dev
watcher failed to start.

* [v2] Improve killing and cleanup of debug binary

- Make sure to kill the debug process in all cases even if an error occured.
- Make sure to kill the last started binary process

v2/cmd/wails/internal/commands/dev/dev.go

@@ -2,6 +2,7 @@ package dev
 
 import (
 	"context"
+	"errors"
 	"fmt"
 	"io"
 	"net"
@@ -168,8 +169,6 @@ func AddSubcommand(app *clir.Cli, w io.Writer) error {
 		buildOptions.Logger = logger
 		buildOptions.UserTags = internal.ParseUserTags(flags.tags)
 
-		var debugBinaryProcess *process.Process = nil
-
 		// Setup signal handler
 		quitChannel := make(chan os.Signal, 1)
 		signal.Notify(quitChannel, os.Interrupt, os.Kill, syscall.SIGTERM)
@@ -177,19 +176,23 @@ func AddSubcommand(app *clir.Cli, w io.Writer) error {
 
 		// Do initial build
 		logger.Println("Building application for development...")
-		newProcess, appBinary, err := restartApp(buildOptions, debugBinaryProcess, flags, exitCodeChannel)
+		debugBinaryProcess, appBinary, err := restartApp(buildOptions, nil, flags, exitCodeChannel)
 		if err != nil {
 			return err
 		}
-		if newProcess != nil {
-			debugBinaryProcess = newProcess
-		}
+		defer func() {
+			if err := killProcessAndCleanupBinary(debugBinaryProcess, appBinary); err != nil {
+				LogDarkYellow("Unable to kill process and cleanup binary: %s", err)
+			}
+		}()
 
 		// frontend:dev:watcher command.
 		if command := projectConfig.DevWatcherCommand; command != "" {
-			var devCommandWaitGroup sync.WaitGroup
-			closer := runFrontendDevWatcherCommand(cwd, command, &devCommandWaitGroup)
-			defer closer(&devCommandWaitGroup)
+			closer, err := runFrontendDevWatcherCommand(cwd, command)
+			if err != nil {
+				return err
+			}
+			defer closer()
 		}
 
 		// open browser
@@ -221,22 +224,17 @@ func AddSubcommand(app *clir.Cli, w io.Writer) error {
 		LogGreen("Using reload debounce setting of %d milliseconds", flags.debounceMS)
 
 		// Watch for changes and trigger restartApp()
-		doWatcherLoop(buildOptions, debugBinaryProcess, flags, watcher, exitCodeChannel, quitChannel, devServerURL)
+		debugBinaryProcess = doWatcherLoop(buildOptions, debugBinaryProcess, flags, watcher, exitCodeChannel, quitChannel, devServerURL)
 
-		// Kill the current program if running
-		if debugBinaryProcess != nil {
-			err := debugBinaryProcess.Kill()
-			if err != nil {
-				return err
-			}
-		}
-
-		// Remove dev binary
-		err = os.Remove(appBinary)
-		if err != nil {
+		// Kill the current program if running and remove dev binary
+		if err := killProcessAndCleanupBinary(debugBinaryProcess, appBinary); err != nil {
 			return err
 		}
 
+		// Reset the process and the binary so the defer knows about it and is a nop.
+		debugBinaryProcess = nil
+		appBinary = ""
+
 		LogGreen("Development mode exited")
 
 		return nil
@@ -244,6 +242,22 @@ func AddSubcommand(app *clir.Cli, w io.Writer) error {
 	return nil
 }
 
+func killProcessAndCleanupBinary(process *process.Process, binary string) error {
+	if process != nil && process.Running {
+		if err := process.Kill(); err != nil {
+			return err
+		}
+	}
+
+	if binary != "" {
+		err := os.Remove(binary)
+		if err != nil && !errors.Is(err, os.ErrNotExist) {
+			return err
+		}
+	}
+	return nil
+}
+
 func syncGoModVersion(cwd string) error {
 	gomodFilename := filepath.Join(cwd, "go.mod")
 	gomodData, err := os.ReadFile(gomodFilename)
@@ -388,35 +402,39 @@ func loadAndMergeProjectConfig(cwd string, flags *devFlags) (*project.Project, e
 }
 
 // runFrontendDevWatcherCommand will run the `frontend:dev:watcher` command if it was given, ex- `npm run dev`
-func runFrontendDevWatcherCommand(cwd string, devCommand string, wg *sync.WaitGroup) func(group *sync.WaitGroup) {
-	LogGreen("Running frontend dev watcher command: '%s'", devCommand)
+func runFrontendDevWatcherCommand(cwd string, devCommand string) (func(), error) {
 	ctx, cancel := context.WithCancel(context.Background())
 	dir := filepath.Join(cwd, "frontend")
 	cmdSlice := strings.Split(devCommand, " ")
-	wg.Add(1)
 	cmd := exec.CommandContext(ctx, cmdSlice[0], cmdSlice[1:]...)
 	cmd.Stderr = os.Stderr
 	cmd.Stdout = os.Stdout
 	cmd.Dir = dir
-
 	setParentGID(cmd)
-	go func(ctx context.Context, devCommand string, cwd string, wg *sync.WaitGroup) {
-		err := cmd.Run()
-		if err != nil {
+	if err := cmd.Start(); err != nil {
+		cancel()
+		return nil, fmt.Errorf("Unable to start frontend DevWatcher: %w", err)
+	}
+
+	LogGreen("Running frontend DevWatcher command: '%s'", devCommand)
+	var wg sync.WaitGroup
+	wg.Add(1)
+	go func() {
+		if err := cmd.Wait(); err != nil {
 			if err.Error() != "exit status 1" {
-				LogRed("Error from '%s': %s", devCommand, err.Error())
+				LogRed("Error from DevWatcher '%s': %s", devCommand, err.Error())
 			}
 		}
-		LogGreen("Dev command exited!")
+		LogGreen("DevWatcher command exited!")
 		wg.Done()
-	}(ctx, devCommand, cwd, wg)
+	}()
 
-	return func(wg *sync.WaitGroup) {
+	return func() {
 		killProc(cmd, devCommand)
-		LogGreen("Dev command killed!")
+		LogGreen("DevWatcher command killed!")
 		cancel()
 		wg.Wait()
-	}
+	}, nil
 }
 
 // initialiseWatcher creates the project directory watcher that will trigger recompile
@@ -463,8 +481,13 @@ func restartApp(buildOptions *build.Options, debugBinaryProcess *process.Process
 	appBinary, err := build.Build(buildOptions)
 	println()
 	if err != nil {
-		LogRed("Build error - continuing to run current version")
-		LogDarkYellow(err.Error())
+		LogRed("Build error - " + err.Error())
+
+		msg := "Continuing to run current version"
+		if debugBinaryProcess == nil {
+			msg = "No version running, build will be retriggered as soon as changes have been detected"
+		}
+		LogDarkYellow(msg)
 		return nil, "", nil
 	}
 
@@ -510,12 +533,8 @@ func restartApp(buildOptions *build.Options, debugBinaryProcess *process.Process
 }
 
 // doWatcherLoop is the main watch loop that runs while dev is active
-func doWatcherLoop(buildOptions *build.Options, debugBinaryProcess *process.Process, flags devFlags, watcher *fsnotify.Watcher, exitCodeChannel chan int, quitChannel chan os.Signal, devServerURL *url.URL) {
+func doWatcherLoop(buildOptions *build.Options, debugBinaryProcess *process.Process, flags devFlags, watcher *fsnotify.Watcher, exitCodeChannel chan int, quitChannel chan os.Signal, devServerURL *url.URL) *process.Process {
 	// Main Loop
-	var (
-		err              error
-		newBinaryProcess *process.Process
-	)
 	var extensionsThatTriggerARebuild = sliceToMap(strings.Split(flags.extensions, ","))
 	var dirsThatTriggerAReload []string
 	for _, dir := range strings.Split(flags.reloadDirs, ",") {
@@ -601,7 +620,7 @@ func doWatcherLoop(buildOptions *build.Options, debugBinaryProcess *process.Proc
 				rebuild = false
 				LogGreen("[Rebuild triggered] files updated")
 				// Try and build the app
-				newBinaryProcess, _, err = restartApp(buildOptions, debugBinaryProcess, flags, exitCodeChannel)
+				newBinaryProcess, _, err := restartApp(buildOptions, debugBinaryProcess, flags, exitCodeChannel)
 				if err != nil {
 					LogRed("Error during build: %s", err.Error())
 					continue
@@ -647,7 +666,7 @@ func doWatcherLoop(buildOptions *build.Options, debugBinaryProcess *process.Proc
 			}
 			if reload {
 				reload = false
-				_, err = http.Get(reloadURL)
+				_, err := http.Get(reloadURL)
 				if err != nil {
 					LogRed("Error during refresh: %s", err.Error())
 				}
@@ -657,6 +676,7 @@ func doWatcherLoop(buildOptions *build.Options, debugBinaryProcess *process.Proc
 			quit = true
 		}
 	}
+	return debugBinaryProcess
 }
 
 func joinPath(url *url.URL, subPath string) string {

v2/cmd/wails/internal/commands/dev/dev_other.go

@@ -6,6 +6,8 @@ package dev
 import (
 	"os/exec"
 	"syscall"
+
+	"golang.org/x/sys/unix"
 )
 
 func setParentGID(cmd *exec.Cmd) {
@@ -15,6 +17,10 @@ func setParentGID(cmd *exec.Cmd) {
 }
 
 func killProc(cmd *exec.Cmd, devCommand string) {
+	if cmd == nil || cmd.Process == nil {
+		return
+	}
+
 	// Experiencing the same issue on macOS BigSur
 	// I'm using Vite, but I would imagine this could be an issue with Node (npm) in general
 	// Also, after several edit/rebuild cycles any abnormal shutdown (crash or CTRL-C) may still leave Node running
@@ -22,7 +28,7 @@ func killProc(cmd *exec.Cmd, devCommand string) {
 	// Not tested on *nix
 	pgid, err := syscall.Getpgid(cmd.Process.Pid)
 	if err == nil {
-		err := syscall.Kill(-pgid, 15) // note the minus sign
+		err := syscall.Kill(-pgid, unix.SIGTERM) // note the minus sign
 		if err != nil {
 			LogRed("Error from '%s' when attempting to kill the process: %s", devCommand, err.Error())
 		}