fix(asyncloader): more flexible cancellation

Continue to load links synchronously after network request completes

requestmanager/asyncloader/asyncloader.go

@@ -179,12 +179,6 @@ func (abl *AsyncLoader) messageQueueWorker() {
 }
 
 func (lr *loadRequest) handle(abl *AsyncLoader) {
-	_, ok := abl.activeRequests[lr.requestID]
-	if !ok {
-		abl.terminateWithError("No active request", lr.resultChan)
-		returnLoadRequest(lr)
-		return
-	}
 	response, err := abl.loadAttempter(lr.requestID, lr.link)
 	if err != nil {
 		lr.resultChan <- AsyncLoadResult{nil, err}
@@ -198,6 +192,12 @@ func (lr *loadRequest) handle(abl *AsyncLoader) {
 		returnLoadRequest(lr)
 		return
 	}
+	_, ok := abl.activeRequests[lr.requestID]
+	if !ok {
+		abl.terminateWithError("No active request", lr.resultChan)
+		returnLoadRequest(lr)
+		return
+	}
 	abl.pausedRequests = append(abl.pausedRequests, lr)
 }
 

requestmanager/asyncloader/asyncloader_test.go

@@ -13,7 +13,7 @@ import (
 	ipld "github.com/ipld/go-ipld-prime"
 )
 
-func TestAsyncLoadWhenRequestNotInProgress(t *testing.T) {
+func TestAsyncLoadInitialLoadSucceeds(t *testing.T) {
 	ctx := context.Background()
 	ctx, cancel := context.WithTimeout(ctx, 10*time.Millisecond)
 	defer cancel()
@@ -31,46 +31,44 @@ func TestAsyncLoadWhenRequestNotInProgress(t *testing.T) {
 
 	select {
 	case result := <-resultChan:
-		if result.Data != nil {
-			t.Fatal("should not have sent responses")
+		if result.Data == nil {
+			t.Fatal("should have sent a response")
 		}
-		if result.Err == nil {
-			t.Fatal("should have sent an error")
-
+		if result.Err != nil {
+			t.Fatal("should not have sent an error")
 		}
 	case <-ctx.Done():
-		t.Fatal("should have produced result")
+		t.Fatal("should have closed response channel")
 	}
 
-	if callCount > 0 {
-		t.Fatal("should not have attempted to load link but did")
+	if callCount == 0 {
+		t.Fatal("should have attempted to load link but did not")
 	}
 }
 
-func TestAsyncLoadWhenInitialLoadSucceeds(t *testing.T) {
+func TestAsyncLoadInitialLoadFails(t *testing.T) {
 	ctx := context.Background()
 	ctx, cancel := context.WithTimeout(ctx, 10*time.Millisecond)
 	defer cancel()
 	callCount := 0
 	loadAttempter := func(gsmsg.GraphSyncRequestID, ipld.Link) ([]byte, error) {
 		callCount++
-		return testutil.RandomBytes(100), nil
+		return nil, fmt.Errorf("something went wrong")
 	}
 	asyncLoader := New(ctx, loadAttempter)
 	asyncLoader.Startup()
 
 	link := testbridge.NewMockLink()
 	requestID := gsmsg.GraphSyncRequestID(rand.Int31())
-	asyncLoader.StartRequest(requestID)
 	resultChan := asyncLoader.AsyncLoad(requestID, link)
 
 	select {
 	case result := <-resultChan:
-		if result.Data == nil {
-			t.Fatal("should have sent a response")
+		if result.Data != nil {
+			t.Fatal("should not have sent responses")
 		}
-		if result.Err != nil {
-			t.Fatal("should not have sent an error")
+		if result.Err == nil {
+			t.Fatal("should have sent an error")
 		}
 	case <-ctx.Done():
 		t.Fatal("should have closed response channel")
@@ -79,23 +77,26 @@ func TestAsyncLoadWhenInitialLoadSucceeds(t *testing.T) {
 	if callCount == 0 {
 		t.Fatal("should have attempted to load link but did not")
 	}
-}
 
-func TestAsyncLoadInitialLoadFails(t *testing.T) {
+}
+func TestAsyncLoadInitialLoadIndeterminateWhenRequestNotInProgress(t *testing.T) {
 	ctx := context.Background()
 	ctx, cancel := context.WithTimeout(ctx, 10*time.Millisecond)
 	defer cancel()
 	callCount := 0
 	loadAttempter := func(gsmsg.GraphSyncRequestID, ipld.Link) ([]byte, error) {
+		var result []byte
+		if callCount > 0 {
+			result = testutil.RandomBytes(100)
+		}
 		callCount++
-		return nil, fmt.Errorf("something went wrong")
+		return result, nil
 	}
 	asyncLoader := New(ctx, loadAttempter)
 	asyncLoader.Startup()
 
 	link := testbridge.NewMockLink()
 	requestID := gsmsg.GraphSyncRequestID(rand.Int31())
-	asyncLoader.StartRequest(requestID)
 	resultChan := asyncLoader.AsyncLoad(requestID, link)
 
 	select {
@@ -105,13 +106,14 @@ func TestAsyncLoadInitialLoadFails(t *testing.T) {
 		}
 		if result.Err == nil {
 			t.Fatal("should have sent an error")
+
 		}
 	case <-ctx.Done():
-		t.Fatal("should have closed response channel")
+		t.Fatal("should have produced result")
 	}
 
-	if callCount == 0 {
-		t.Fatal("should have attempted to load link but did not")
+	if callCount > 1 {
+		t.Fatal("should have failed after load with indeterminate result")
 	}
 }