Say a Platen job's second page as an error instead of dropping it (issue 1531)

modules/brother/cmd/brother/escl.go

@@ -150,9 +150,13 @@ func (c *Client) Scan(ctx context.Context, resolution int, colorMode string) ([]
 		c.deleteJob(job)
 		return nil, err
 	}
-	if !c.closeJob(ctx, doc) {
+	closed, extra := c.closeJob(ctx, doc)
+	if !closed {
 		c.deleteJob(job)
 	}
+	if extra > 0 {
+		return nil, fmt.Errorf("the scanner gave more pages than the glass holds: %d", 1+extra)
+	}
 	return image, nil
 }
 
@@ -250,31 +254,32 @@ func (c *Client) page(ctx context.Context, doc string) ([]byte, error) {
 }
 
 // maxClosing bounds the NextDocument requests that ask for the end of a job.
-const maxClosing = 3
+const maxClosing = 1
 
 // closeJob asks NextDocument until the scanner answers 404, which ends the job; true when it did (or the job is
-// gone already). A further page is read and dropped, within the bound.
-func (c *Client) closeJob(ctx context.Context, doc string) bool {
+// gone already), and how many further pages it sent instead; any is an error for the caller, for a Platen job holds
+// one page. It stops at the first further page, and the job is then deleted.
+func (c *Client) closeJob(ctx context.Context, doc string) (closed bool, extra int) {
 	ctx, cancel := context.WithTimeout(context.WithoutCancel(ctx), 15*time.Second)
 	defer cancel()
 	for i := 0; i < maxClosing; i++ {
 		resp, err := c.get(ctx, doc)
 		if err != nil {
 			c.logf("brother: closing the scan job: %v", err)
-			return false
+			return false, 0
 		}
 		readBody(resp, maxImage)
 		switch resp.StatusCode {
 		case http.StatusNotFound, http.StatusGone:
-			return true
+			return true, 0
 		case http.StatusOK:
-			continue
+			c.logf("brother: the scanner sent a further page after the glass's one")
+			return false, 1
 		}
 		c.logf("brother: closing the scan job: NextDocument answered %s", resp.Status)
-		return false
+		return false, 0
 	}
-	c.logf("brother: the scanner kept sending pages after %d requests", maxClosing)
-	return false
+	return false, 0
 }
 
 // deleteJob deletes the job, best effort; true when the job is gone. A job that cannot be deleted is remembered, so

modules/brother/cmd/brother/escl_test.go

@@ -203,3 +203,16 @@ func TestABusyScannerWithABrotherJobLeftIsRetriedOnceAfterItCloses(t *testing.T)
 		t.Fatalf("%d posts, want 3 (stuck job, refused, retried)", f.posts)
 	}
 }
+
+// 1531: a second page for a Platen job is an error, never a silently incomplete scan, and the job is closed.
+func TestMorePagesThanTheGlassHoldsIsAnErrorAndTheJobIsClosed(t *testing.T) {
+	f := newFakeScanner(t, fixture(t, "scan.jpg"))
+	f.pages = 2
+	image, err := NewClient(f.base()).Scan(context.Background(), 300, "color")
+	if err == nil || !strings.Contains(err.Error(), "the scanner gave more pages than the glass holds") {
+		t.Fatalf("image %d bytes, error = %v, want more pages than the glass holds", len(image), err)
+	}
+	if f.open || f.deletes != 1 {
+		t.Fatalf("job open %v, %d DELETEs, want closed by 1 DELETE", f.open, f.deletes)
+	}
+}

modules/brother/cmd/brother/helpers_test.go

@@ -33,6 +33,8 @@ type fakeScanner struct {
 	// or the job is DELETEd; the fake does the same.
 	open      bool
 	delivered bool
+	pages     int // pages a job holds beyond the first
+	served    int
 	deletes   int
 	gone      bool // the first NextDocument answers 410 (the job was cancelled at the device)
 	dropConn  bool // the first NextDocument loses its answer
@@ -70,7 +72,7 @@ func newFakeScanner(t *testing.T, image []byte) *fakeScanner {
 			http.Error(w, "busy", http.StatusServiceUnavailable)
 			return
 		}
-		f.open, f.delivered = true, false
+		f.open, f.delivered, f.served = true, false, 0
 		// The captured Location names the real scanner's host; the test's own server stands in for it.
 		w.Header().Set("Location", f.URL+"/eSCL/ScanJobs/7320ead5")
 		w.WriteHeader(http.StatusCreated)
@@ -95,12 +97,13 @@ func newFakeScanner(t *testing.T, image []byte) *fakeScanner {
 			}
 			return
 		}
-		if f.delivered || f.noPage {
+		if f.noPage || (f.delivered && f.served > f.pages) {
 			f.open = false
 			http.NotFound(w, r)
 			return
 		}
 		f.delivered = true
+		f.served++
 		w.Header().Set("Content-Type", "image/jpeg")
 		w.Write(f.image)
 	})