Merge pull request #1693 from alixander/fix-layers-link

cli: Fix invalid link paths

ci/release/changelogs/next.md

@@ -20,3 +20,4 @@
 - Correctly reports errors from invalid values set by globs. [#1691](https://github.com/terrastruct/d2/pull/1691)
 - Fixes panic when spread substitution referenced a nonexistant var. [#1695](https://github.com/terrastruct/d2/pull/1695)
 - Fixes incorrect appendix icon numbering. [#1704](https://github.com/terrastruct/d2/pull/1704)
+- Fixes crash when using `--watch` and navigating to an invalid board path [#1693](https://github.com/terrastruct/d2/pull/1693)

d2cli/fmt.go

@@ -17,7 +17,7 @@ import (
 func fmtCmd(ctx context.Context, ms *xmain.State) (err error) {
 	defer xdefer.Errorf(&err, "failed to fmt")
 
-	ms.Opts = xmain.NewOpts(ms.Env, ms.Log, ms.Opts.Flags.Args()[1:])
+	ms.Opts = xmain.NewOpts(ms.Env, ms.Opts.Flags.Args()[1:])
 	if len(ms.Opts.Args) == 0 {
 		return xmain.UsageErrorf("fmt must be passed at least one file to be formatted")
 	}

d2cli/main.go

@@ -503,7 +503,7 @@ func compile(ctx context.Context, ms *xmain.State, plugins []d2plugin.Plugin, la
 
 		board := diagram.GetBoard(boardPath)
 		if board == nil {
-			return nil, false, fmt.Errorf("Diagram with path %s not found", boardPath)
+			return nil, false, fmt.Errorf(`Diagram with path "%s" not found. Did you mean to specify a board like "layers.%s"?`, boardPath, boardPath)
 		}
 
 		boards, err := render(ctx, ms, compileDur, plugin, renderOpts, inputPath, outputPath, bundle, forceAppendix, page, ruler, board)

d2plugin/plugin_dagre.go

@@ -61,7 +61,7 @@ func (p *dagrePlugin) HydrateOpts(opts []byte) error {
 func (p *dagrePlugin) Info(ctx context.Context) (*PluginInfo, error) {
 	p.mu.Lock()
 	defer p.mu.Unlock()
-	opts := xmain.NewOpts(nil, nil, nil)
+	opts := xmain.NewOpts(nil, nil)
 	flags, err := p.Flags(ctx)
 	if err != nil {
 		return nil, err

d2plugin/plugin_elk.go

@@ -76,7 +76,7 @@ func (p *elkPlugin) HydrateOpts(opts []byte) error {
 }
 
 func (p elkPlugin) Info(ctx context.Context) (*PluginInfo, error) {
-	opts := xmain.NewOpts(nil, nil, nil)
+	opts := xmain.NewOpts(nil, nil)
 	flags, err := p.Flags(ctx)
 	if err != nil {
 		return nil, err

d2target/d2target.go

@@ -74,7 +74,8 @@ type Diagram struct {
 }
 
 // boardPath comes in the form of "x/layers/z/scenarios/a"
-// or in the form of "layers/z/scenarios/a"
+// or "layers/z/scenarios/a"
+// or "x/z/a"
 func (d *Diagram) GetBoard(boardPath string) *Diagram {
 	path := strings.Split(boardPath, string(os.PathSeparator))
 	if len(path) == 0 || len(boardPath) == 0 {
@@ -127,17 +128,17 @@ func (d *Diagram) getBoard(boardPath []string) *Diagram {
 
 	for _, b := range d.Layers {
 		if b.Name == head {
-			return b.getBoard(boardPath[2:])
+			return b.getBoard(boardPath[1:])
 		}
 	}
 	for _, b := range d.Scenarios {
 		if b.Name == head {
-			return b.getBoard(boardPath[2:])
+			return b.getBoard(boardPath[1:])
 		}
 	}
 	for _, b := range d.Steps {
 		if b.Name == head {
-			return b.getBoard(boardPath[2:])
+			return b.getBoard(boardPath[1:])
 		}
 	}
 	return nil

e2etests-cli/main_test.go

@@ -3,12 +3,18 @@ package e2etests_cli
 import (
 	"bytes"
 	"context"
+	"errors"
+	"fmt"
+	"net/http"
 	"os"
 	"path/filepath"
+	"regexp"
 	"strings"
 	"testing"
 	"time"
 
+	"nhooyr.io/websocket"
+
 	"oss.terrastruct.com/util-go/assert"
 	"oss.terrastruct.com/util-go/diff"
 	"oss.terrastruct.com/util-go/xmain"
@@ -544,6 +550,169 @@ i used to read
 				assert.Equal(t, "x -> y\n", string(gotBar))
 			},
 		},
+		{
+			name: "watch-regular",
+			run: func(t *testing.T, ctx context.Context, dir string, env *xos.Env) {
+				writeFile(t, dir, "index.d2", `
+a -> b
+b.link: layers.cream
+
+layers: {
+    cream: {
+        c -> b
+    }
+}`)
+				stderr := &bytes.Buffer{}
+				tms := testMain(dir, env, "--watch", "--browser=0", "index.d2")
+				tms.Stderr = stderr
+
+				tms.Start(t, ctx)
+				defer func() {
+					// Manually close, since watcher is daemon
+					err := tms.Signal(ctx, os.Interrupt)
+					assert.Success(t, err)
+				}()
+
+				// Wait for watch server to spin up and listen
+				urlRE := regexp.MustCompile(`127.0.0.1:([0-9]+)`)
+				watchURL := waitLogs(ctx, stderr, urlRE)
+
+				if watchURL == "" {
+					t.Error(errors.New(stderr.String()))
+				}
+				stderr.Reset()
+
+				// Start a client
+				c, _, err := websocket.Dial(ctx, fmt.Sprintf("ws://%s/watch", watchURL), nil)
+				assert.Success(t, err)
+				defer c.CloseNow()
+
+				// Get the link
+				_, msg, err := c.Read(ctx)
+				assert.Success(t, err)
+				aRE := regexp.MustCompile(`href=\\"([^\"]*)\\"`)
+				match := aRE.FindSubmatch(msg)
+				assert.Equal(t, 2, len(match))
+				linkedPath := match[1]
+
+				err = getWatchPage(ctx, t, fmt.Sprintf("http://%s/%s", watchURL, linkedPath))
+				assert.Success(t, err)
+
+				successRE := regexp.MustCompile(`broadcasting update to 1 client`)
+				line := waitLogs(ctx, stderr, successRE)
+				assert.NotEqual(t, "", line)
+			},
+		},
+		{
+			name: "watch-ok-link",
+			run: func(t *testing.T, ctx context.Context, dir string, env *xos.Env) {
+				// This link technically works because D2 interprets it as a URL,
+				// and on local filesystem, that is whe path where the compilation happens
+				// to output it to.
+				writeFile(t, dir, "index.d2", `
+a -> b
+b.link: cream
+
+layers: {
+    cream: {
+        c -> b
+    }
+}`)
+				stderr := &bytes.Buffer{}
+				tms := testMain(dir, env, "--watch", "--browser=0", "index.d2")
+				tms.Stderr = stderr
+
+				tms.Start(t, ctx)
+				defer func() {
+					// Manually close, since watcher is daemon
+					err := tms.Signal(ctx, os.Interrupt)
+					assert.Success(t, err)
+				}()
+
+				// Wait for watch server to spin up and listen
+				urlRE := regexp.MustCompile(`127.0.0.1:([0-9]+)`)
+				watchURL := waitLogs(ctx, stderr, urlRE)
+
+				if watchURL == "" {
+					t.Error(errors.New(stderr.String()))
+				}
+				stderr.Reset()
+
+				// Start a client
+				c, _, err := websocket.Dial(ctx, fmt.Sprintf("ws://%s/watch", watchURL), nil)
+				assert.Success(t, err)
+				defer c.CloseNow()
+
+				// Get the link
+				_, msg, err := c.Read(ctx)
+				assert.Success(t, err)
+				aRE := regexp.MustCompile(`href=\\"([^\"]*)\\"`)
+				match := aRE.FindSubmatch(msg)
+				assert.Equal(t, 2, len(match))
+				linkedPath := match[1]
+
+				err = getWatchPage(ctx, t, fmt.Sprintf("http://%s/%s", watchURL, linkedPath))
+				assert.Success(t, err)
+
+				successRE := regexp.MustCompile(`broadcasting update to 1 client`)
+				line := waitLogs(ctx, stderr, successRE)
+				assert.NotEqual(t, "", line)
+			},
+		},
+		{
+			name: "watch-bad-link",
+			run: func(t *testing.T, ctx context.Context, dir string, env *xos.Env) {
+				// Just verify we don't crash even with a bad link (it's treated as a URL, which users might have locally)
+				writeFile(t, dir, "index.d2", `
+a -> b
+b.link: dream
+
+layers: {
+    cream: {
+        c -> b
+    }
+}`)
+				stderr := &bytes.Buffer{}
+				tms := testMain(dir, env, "--watch", "--browser=0", "index.d2")
+				tms.Stderr = stderr
+
+				tms.Start(t, ctx)
+				defer func() {
+					// Manually close, since watcher is daemon
+					err := tms.Signal(ctx, os.Interrupt)
+					assert.Success(t, err)
+				}()
+
+				// Wait for watch server to spin up and listen
+				urlRE := regexp.MustCompile(`127.0.0.1:([0-9]+)`)
+				watchURL := waitLogs(ctx, stderr, urlRE)
+
+				if watchURL == "" {
+					t.Error(errors.New(stderr.String()))
+				}
+				stderr.Reset()
+
+				// Start a client
+				c, _, err := websocket.Dial(ctx, fmt.Sprintf("ws://%s/watch", watchURL), nil)
+				assert.Success(t, err)
+				defer c.CloseNow()
+
+				// Get the link
+				_, msg, err := c.Read(ctx)
+				assert.Success(t, err)
+				aRE := regexp.MustCompile(`href=\\"([^\"]*)\\"`)
+				match := aRE.FindSubmatch(msg)
+				assert.Equal(t, 2, len(match))
+				linkedPath := match[1]
+
+				err = getWatchPage(ctx, t, fmt.Sprintf("http://%s/%s", watchURL, linkedPath))
+				assert.Success(t, err)
+
+				successRE := regexp.MustCompile(`broadcasting update to 1 client`)
+				line := waitLogs(ctx, stderr, successRE)
+				assert.NotEqual(t, "", line)
+			},
+		},
 	}
 
 	ctx := context.Background()
@@ -640,3 +809,39 @@ func testdataIgnoreDiff(tb testing.TB, ext string, got []byte) {
 func getNumBoards(svg string) int {
 	return strings.Count(svg, `class="d2`)
 }
+
+func waitLogs(ctx context.Context, buf *bytes.Buffer, pattern *regexp.Regexp) string {
+	ticker := time.NewTicker(10 * time.Millisecond)
+	defer ticker.Stop()
+	var match string
+	for i := 0; i < 100 && match == ""; i++ {
+		select {
+		case <-ticker.C:
+			out := buf.String()
+			match = pattern.FindString(out)
+		case <-ctx.Done():
+			ticker.Stop()
+			return ""
+		}
+	}
+
+	return match
+}
+
+func getWatchPage(ctx context.Context, t *testing.T, page string) error {
+	req, err := http.NewRequestWithContext(ctx, "GET", page, nil)
+	if err != nil {
+		return err
+	}
+
+	var httpClient = &http.Client{}
+	resp, err := httpClient.Do(req)
+	if err != nil {
+		return err
+	}
+	defer resp.Body.Close()
+	if resp.StatusCode != 200 {
+		return fmt.Errorf("status code: %d", resp.StatusCode)
+	}
+	return nil
+}