service: Return the error rather than printing it

This enables callers to make the difference between a successful
service call and a failed one.

downstream.go

@@ -2428,7 +2428,7 @@ func (dc *downstreamConn) handleMessageRegistered(ctx context.Context, msg *irc.
 					})
 				}
 				if msg.Command == "PRIVMSG" {
-					handleServicePRIVMSG(&serviceContext{
+					if err := handleServicePRIVMSG(&serviceContext{
 						Context: ctx,
 						nick:    dc.nick,
 						network: dc.network,
@@ -2438,7 +2438,9 @@ func (dc *downstreamConn) handleMessageRegistered(ctx context.Context, msg *irc.
 						print: func(text string) {
 							sendServicePRIVMSG(dc, text)
 						},
-					}, text)
+					}, text); err != nil {
+						sendServicePRIVMSG(dc, fmt.Sprintf("error: %v", err))
+					}
 				}
 				continue
 			}

service.go

@@ -125,28 +125,24 @@ func splitWords(s string) ([]string, error) {
 	return words, nil
 }
 
-func handleServicePRIVMSG(ctx *serviceContext, text string) {
+func handleServicePRIVMSG(ctx *serviceContext, text string) error {
 	words, err := splitWords(text)
 	if err != nil {
-		ctx.print(fmt.Sprintf(`error: failed to parse command: %v`, err))
-		return
+		return fmt.Errorf(`failed to parse command: %v`, err)
 	}
-	handleServiceCommand(ctx, words)
+	return handleServiceCommand(ctx, words)
 }
 
-func handleServiceCommand(ctx *serviceContext, words []string) {
+func handleServiceCommand(ctx *serviceContext, words []string) error {
 	cmd, params, err := serviceCommands.Get(words)
 	if err != nil {
-		ctx.print(fmt.Sprintf(`error: %v (type "help" for a list of commands)`, err))
-		return
+		return fmt.Errorf(`%v (type "help" for a list of commands)`, err)
 	}
 	if cmd.admin && !ctx.admin {
-		ctx.print("error: you must be an admin to use this command")
-		return
+		return fmt.Errorf("you must be an admin to use this command")
 	}
 	if !cmd.global && ctx.user == nil {
-		ctx.print("error: this command must be run as a user (try running with user run)")
-		return
+		return fmt.Errorf("this command must be run as a user (try running with user run)")
 	}
 
 	if cmd.handle == nil {
@@ -154,7 +150,8 @@ func handleServiceCommand(ctx *serviceContext, words []string) {
 			var l []string
 			appendServiceCommandSetHelp(cmd.children, words, ctx.admin, ctx.user == nil, &l)
 			ctx.print("available commands: " + strings.Join(l, ", "))
-		} else {
+			return nil
+		}
 		// Pretend the command does not exist if it has neither children nor handler.
 		// This is obviously a bug but it is better to not die anyway.
 		var logger Logger
@@ -164,14 +161,10 @@ func handleServiceCommand(ctx *serviceContext, words []string) {
 			logger = ctx.srv.Logger
 		}
 		logger.Printf("command without handler and subcommands invoked:", words[0])
-			ctx.print(fmt.Sprintf("command %q not found", words[0]))
-		}
-		return
+		return fmt.Errorf("command %q not found", words[0])
 	}
 
-	if err := cmd.handle(ctx, params); err != nil {
-		ctx.print(fmt.Sprintf("error: %v", err))
-	}
+	return cmd.handle(ctx, params)
 }
 
 func (cmds serviceCommandSet) Get(params []string) (*serviceCommand, []string, error) {
@@ -1169,8 +1162,7 @@ func handleUserRun(ctx *serviceContext, params []string) error {
 	username := params[0]
 	params = params[1:]
 	if ctx.user != nil && username == ctx.user.Username {
-		handleServiceCommand(ctx, params)
-		return nil
+		return handleServiceCommand(ctx, params)
 	}
 
 	u := ctx.srv.getUser(username)
@@ -1179,9 +1171,11 @@ func handleUserRun(ctx *serviceContext, params []string) error {
 	}
 
 	printCh := make(chan string, 1)
+	retCh := make(chan error, 1)
 	ev := eventUserRun{
 		params: params,
 		print:  printCh,
+		ret:    retCh,
 	}
 	select {
 	case <-ctx.Done():
@@ -1199,13 +1193,14 @@ func handleUserRun(ctx *serviceContext, params []string) error {
 			// in case the event is never processed.
 			// TODO: Properly fix this condition by flushing the u.events queue
 			//       and running close(ev.print) in a defer
-			return nil
+			return fmt.Errorf("timeout executing command")
 		case text, ok := <-printCh:
-			if !ok {
-				return nil
-			}
+			if ok {
 				ctx.print(text)
 			}
+		case ret := <-retCh:
+			return ret
+		}
 	}
 }
 

user.go

@@ -87,6 +87,7 @@ type eventTryRegainNick struct {
 type eventUserRun struct {
 	params []string
 	print  chan string
+	ret    chan error
 }
 
 type deliveredClientMap map[string]string // client name -> msg ID
@@ -808,7 +809,7 @@ func (u *user) run() {
 			e.uc.tryRegainNick(e.nick)
 		case eventUserRun:
 			ctx := context.TODO()
-			handleServiceCommand(&serviceContext{
+			err := handleServiceCommand(&serviceContext{
 				Context: ctx,
 				user:    u,
 				srv:     u.srv,
@@ -823,7 +824,10 @@ func (u *user) run() {
 					}
 				},
 			}, e.params)
-			close(e.print)
+			select {
+			case <-ctx.Done():
+			case e.ret <- err:
+			}
 		case eventStop:
 			for _, dc := range u.downstreamConns {
 				dc.Close()