fix: remote NLP endpoint failures surface as errors instead of panicking

cmd/vale/command.go

@@ -258,8 +258,11 @@ func runTag(args []string, _ *core.CLIFlags) error {
 		return err
 	}
 
-	out := core.TextToContext(
+	out, err := core.TextToContext(
 		string(text), &nlp.Info{Lang: args[1], Endpoint: args[2]})
+	if err != nil {
+		return err
+	}
 
 	return printJSON(out)
 }

internal/core/util.go

@@ -265,14 +265,19 @@ func SplitLines(data []byte, atEOF bool) (adv int, token []byte, err error) { //
 	return 0, nil, nil
 }
 
-func TextToContext(text string, meta *nlp.Info) []nlp.TaggedWord {
+func TextToContext(text string, meta *nlp.Info) ([]nlp.TaggedWord, error) {
 	context := []nlp.TaggedWord{}
 
 	for idx, line := range strings.Split(text, "\n") {
 		plain := stripMarkdown(line)
 
+		toks, err := nlp.TextToTokens(plain, meta)
+		if err != nil {
+			return nil, err
+		}
+
 		pos := 0
-		for _, tok := range nlp.TextToTokens(plain, meta) {
+		for _, tok := range toks {
 			if strings.TrimSpace(tok.Text) != "" {
 				s := strings.Index(line[pos:], tok.Text) + len(line[:pos])
 				if !StringInSlice(tok.Tag, []string{"''", "``"}) {
@@ -288,7 +293,7 @@ func TextToContext(text string, meta *nlp.Info) []nlp.TaggedWord {
 		}
 	}
 
-	return context
+	return context, nil
 }
 
 func ReplaceAllStringSubmatchFunc(re *regexp.Regexp, str string, repl func([]string) string) string {

internal/core/util_test.go

@@ -1,11 +1,14 @@
 package core
 
 import (
+	"net/http"
+	"net/http/httptest"
 	"os"
 	"path/filepath"
 	"reflect"
 	"testing"
 
+	"github.com/vale-cli/vale/v3/internal/nlp"
 	"github.com/vale-cli/vale/v3/internal/system"
 )
 
@@ -39,6 +42,39 @@ func TestFormatFromExt(t *testing.T) {
 	}
 }
 
+// TextToContext is the production caller of nlp.TextToTokens (reached from
+// the `tag` CLI command, cmd/vale/command.go's runTag) -- it used to panic
+// when a configured remote endpoint's /tag request failed, crashing the
+// whole vale process instead of letting runTag's already-existing `if err
+// != nil { return err }` handling report it normally, the same way
+// Info.Compute's fix reused lintProse's own pre-existing error handling.
+// This confirms TextToContext now returns a clean error instead.
+func TestTextToContextReturnsErrorOnTagEndpointFailure(t *testing.T) {
+	server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) {
+		w.WriteHeader(http.StatusInternalServerError)
+		_, _ = w.Write([]byte(`{"Tokens":[]}`))
+	}))
+	defer server.Close()
+
+	var out []nlp.TaggedWord
+	var err error
+	func() {
+		defer func() {
+			if p := recover(); p != nil {
+				t.Fatalf("TextToContext panicked instead of returning an error: %v", p)
+			}
+		}()
+		out, err = TextToContext("some text", &nlp.Info{Lang: "id", Endpoint: server.URL})
+	}()
+
+	if err == nil {
+		t.Fatalf("TextToContext returned a nil error for a failed /tag request, want a non-nil error")
+	}
+	if out != nil {
+		t.Errorf("context = %v, want nil alongside the error", out)
+	}
+}
+
 func TestPrepText(t *testing.T) {
 	rawToPrepped := map[string]string{
 		"foo\r\nbar":     "foo\nbar",

internal/nlp/http.go

@@ -2,6 +2,7 @@ package nlp
 
 import (
 	"encoding/json"
+	"fmt"
 	"io"
 	"net/http"
 	"net/url"
@@ -17,6 +18,15 @@ type TagResult struct {
 	Tokens []tag.Token
 }
 
+// post sends the request and returns its body, but only for a successful
+// (2xx) response.
+//
+// Without this check, a remote endpoint returning e.g. `500 {"sents":[]}` --
+// an error status with a technically-valid-but-degenerate JSON body -- was
+// decoded exactly as if it had succeeded: doSegment or pos would hand back a
+// zero-value result and a nil error, and a caller reading that as "no
+// sentences" or "no tokens" rather than "the request failed" would silently
+// carry on with wrong data instead of surfacing the real problem.
 func post(url string) ([]byte, error) {
 	var body []byte
 
@@ -31,6 +41,10 @@ func post(url string) ([]byte, error) {
 		return body, err
 	}
 
+	if resp.StatusCode < 200 || resp.StatusCode >= 300 {
+		return nil, fmt.Errorf("nlp: request to %s failed: %s", url, resp.Status)
+	}
+
 	return body, nil
 }
 

internal/nlp/http_test.go

@@ -0,0 +1,114 @@
+package nlp
+
+import (
+	"net/http"
+	"net/http/httptest"
+	"strings"
+	"testing"
+
+	"github.com/jdkato/prose/v3/tag"
+)
+
+// A remote endpoint returning a non-2xx status with an otherwise
+// well-formed JSON body -- e.g. `500 {"sents":[]}` -- must not be read as a
+// successful, empty result. Without a status check, json.Unmarshal has
+// nothing to fail on: the caller gets a nil error and a zero-value result,
+// indistinguishable from "the endpoint really has nothing to report" --
+// silently wrong instead of a surfaced failure.
+func TestPostRejectsNonTwoXXStatus(t *testing.T) {
+	server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) {
+		w.WriteHeader(http.StatusInternalServerError)
+		_, _ = w.Write([]byte(`{"sents":[]}`))
+	}))
+	defer server.Close()
+
+	body, err := post(server.URL)
+	if err == nil {
+		t.Fatalf("post returned a nil error for a %d response with a valid JSON body",
+			http.StatusInternalServerError)
+	}
+	if body != nil {
+		t.Errorf("body = %q, want nil alongside the error", body)
+	}
+}
+
+// post is the shared transport both doSegment (/segment) and pos (/tag) call
+// through, so a status check there has to cover both -- this confirms it
+// does for /segment.
+func TestDoSegmentReturnsErrorOnNonTwoXXStatus(t *testing.T) {
+	server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) {
+		w.WriteHeader(http.StatusInternalServerError)
+		_, _ = w.Write([]byte(`{"Sents":[]}`))
+	}))
+	defer server.Close()
+
+	if _, err := doSegment("some text", "id", server.URL); err == nil {
+		t.Fatalf("doSegment returned a nil error for a %d /segment response",
+			http.StatusInternalServerError)
+	}
+}
+
+// Same check for /tag, the tagging counterpart to /segment.
+func TestPosReturnsErrorOnNonTwoXXStatus(t *testing.T) {
+	server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) {
+		w.WriteHeader(http.StatusInternalServerError)
+		_, _ = w.Write([]byte(`{"Tokens":[]}`))
+	}))
+	defer server.Close()
+
+	if _, err := pos("some text", "id", server.URL); err == nil {
+		t.Fatalf("pos returned a nil error for a %d /tag response",
+			http.StatusInternalServerError)
+	}
+}
+
+// TextToTokens used to panic when a configured remote endpoint's /tag
+// request failed -- a network error, a non-2xx status, a malformed response
+// -- which crashed the whole vale process. Its only production caller,
+// TextToContext (internal/core/util.go, reached from the `tag` CLI command),
+// already had somewhere sensible to route an error: this confirms
+// TextToTokens itself now returns one instead of panicking, matching the
+// same fix already made to Info.Compute (see
+// TestComputeReturnsErrorOnSegmentEndpointFailure in provider_test.go).
+func TestTextToTokensReturnsErrorOnTagEndpointFailure(t *testing.T) {
+	server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) {
+		w.WriteHeader(http.StatusInternalServerError)
+		_, _ = w.Write([]byte(`{"Tokens":[]}`))
+	}))
+	defer server.Close()
+
+	var toks []tag.Token
+	var err error
+	func() {
+		defer func() {
+			if p := recover(); p != nil {
+				t.Fatalf("TextToTokens panicked instead of returning an error: %v", p)
+			}
+		}()
+		toks, err = TextToTokens("some text", &Info{Lang: "id", Endpoint: server.URL})
+	}()
+
+	if err == nil {
+		t.Fatalf("TextToTokens returned a nil error for a failed /tag request, want a non-nil error")
+	}
+	if toks != nil {
+		t.Errorf("tokens = %v, want nil alongside the error", toks)
+	}
+}
+
+// Control: an ordinary 2xx response must still be read normally.
+func TestPostSucceedsOnTwoXXStatus(t *testing.T) {
+	server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) {
+		w.WriteHeader(http.StatusOK)
+		_, _ = w.Write([]byte(`{"sents":["ok"]}`))
+	}))
+	defer server.Close()
+
+	body, err := post(server.URL)
+	if err != nil {
+		t.Fatalf("post returned an error for a 200 response: %v", err)
+	}
+	if !strings.Contains(string(body), "ok") {
+		t.Errorf("body = %q, want it to contain the response payload", body)
+	}
+}

internal/nlp/prose.go

@@ -172,17 +172,26 @@ func tagTextWith(model, text string) ([]tag.Token, error) {
 // text[tok.Start:tok.Start+len(tok.Text)] == tok.Text. Tokens from a remote
 // NLP endpoint do not: that API returns text and tags only, so Start is zero
 // throughout and callers needing positions must locate the tokens themselves.
-func TextToTokens(text string, nlp *Info) []tag.Token {
+//
+// A remote endpoint's request can fail -- a network error, a timeout, a
+// non-2xx status (see post, in http.go) -- so this reports that as a real
+// error rather than panicking: there is no recover() anywhere in Vale, so a
+// panic here would crash the whole run instead of surfacing as a normal,
+// reportable error. TextToContext (internal/core/util.go) and its own
+// caller, the `tag` CLI command (cmd/vale/command.go's runTag), already
+// return errors the same way; both are updated to thread this one through
+// rather than let it panic.
+func TextToTokens(text string, nlp *Info) ([]tag.Token, error) {
 	// Determine if (and how) we need to do POS tagging.
 	if nlp == nil || nlp.Endpoint == "" {
 		// Fall back to our internal library (English-only).
-		return tagText(text)
+		return tagText(text), nil
 	}
 	result, err := pos(text, nlp.Lang, nlp.Endpoint)
 	if err != nil {
-		panic(err)
+		return nil, err
 	}
-	return result.Tokens
+	return result.Tokens, nil
 }
 
 // textToTokensWith converts text to tagged tokens with the named tagger.

internal/nlp/provider.go

@@ -175,18 +175,42 @@ type Info struct {
 // must not reach them. See #1132.
 func (n *Info) Compute(block *Block, split bool) ([]Block, error) {
 	seg := SentenceTokenizer.Segment
+
+	// A remote endpoint's segmentation request can fail -- a network error,
+	// a timeout, a non-2xx status (see post, in http.go) -- and Compute runs
+	// during block construction, ahead of every rule's own Run. There is no
+	// recover() anywhere in Vale, so panicking here would crash the whole
+	// run instead of surfacing as this one file's lint error the way
+	// lintProse already reports any other error Compute returns (wrapped in
+	// core.NewE100; see internal/lint/lint.go). seg itself has to keep
+	// returning []string -- it is also the plain, error-free local
+	// segmenter -- so a remote failure is captured here and turned into
+	// Compute's own returned error once doNLP is done calling it, rather
+	// than changing seg's signature for this one caller.
+	var segErr error
 	if n.Endpoint != "" && n.Lang != "en" {
 		// We only use external segmentation for non-English text since prose
 		// (our native library) is more efficient.
 		seg = func(text string) []string {
 			ret, err := doSegment(text, n.Lang, n.Endpoint)
 			if err != nil {
-				panic(err)
+				segErr = err
+				return nil
 			}
 			return ret.Sents
 		}
 	}
-	return n.doNLP(block, seg, split)
+
+	blks := n.doNLP(block, seg, split)
+	if segErr != nil {
+		// The request failed partway through block construction: whatever
+		// doNLP built around the failed call is incomplete, not merely
+		// missing a few sentences, so it is discarded rather than returned
+		// alongside the error.
+		return nil, segErr
+	}
+
+	return blks, nil
 }
 
 // offsetOf locates piece within blk.Text and returns its offset in blk's
@@ -218,7 +242,7 @@ func offsetOf(blk *Block, base int, piece string, cursor *int) (int, int) {
 	return start, base + start
 }
 
-func (n *Info) doNLP(blk *Block, seg segmenter, split bool) ([]Block, error) {
+func (n *Info) doNLP(blk *Block, seg segmenter, split bool) []Block {
 	blks := []Block{}
 
 	ctx := blk.Context
@@ -252,5 +276,5 @@ func (n *Info) doNLP(blk *Block, seg segmenter, split bool) ([]Block, error) {
 	blks = append(
 		blks, NewLinedBlock(ctx, blk.Text, blk.Scope, idx).at(base).withRuns(blk.Runs, 0))
 
-	return blks, nil
+	return blks
 }