async-await: Move to completed state before cancelling task during finish()

[simonjbeaumont]

As reported in #1299 and #1301, there are some scenarios where the closing the channel or using the response stream writer after the channel has been closed will lead to a crash.

Specifically, when finish() is called the state was not progressed to .completed before cancelling the task. This was to maintain parity with the ELG-based API where the status and the trailers were still sent after finish() is called. We now believe this to be misguided and we shouldn't expect to be able to send anything on the channel at this point because we are tearing the handler and the channel down.

This changes finish() to move to the .completed state before cancelling the userHandlerTask. As a result, when the completion handler for the user function fires, it will call handleError(_:) with CancellationError (as before) but now the error handler will not attempt to send the status or trailers back via the interceptors because the state will be in .completed.

Tests for receiving an error after headers and after a message have been added.

[simonjbeaumont]

This should fix #1301

[simonjbeaumont]

This should fix #1299

[simonjbeaumont]

@glbrntt what do we want to do about the deinit check here:

// AsyncWriter.swift

  deinit {
    switch self._completionState {
    case .completed:
      ()
    case .incomplete, .pending:
      assertionFailure("writer has not completed is pending completion")
    }
  }

This will currently cause an assertionFailure if I add the following test to the integration tests, which shuts down the server before completing the RPC.

diff --git a/Tests/GRPCTests/AsyncAwaitSupport/AsyncIntegrationTests.swift b/Tests/GRPCTests/AsyncAwaitSupport/AsyncIntegrationTests.swift
index bc5f7eb4..298055cb 100644
--- a/Tests/GRPCTests/AsyncAwaitSupport/AsyncIntegrationTests.swift
+++ b/Tests/GRPCTests/AsyncAwaitSupport/AsyncIntegrationTests.swift
@@ -197,2 +197,11 @@ final class AsyncIntegrationTests: GRPCTestCase {
   }
+
+  func testServerCloseAfterMessage() {
+    XCTAsyncTest {
+      let update = self.echo.makeUpdateCall()
+      try await update.requestStream.send(.with { $0.text = "hello" })
+      _ = try await update.responses.first(where: { _ in true })
+      // server is closed in tearDown.
+    }
+  }
 }

[glbrntt]

@glbrntt what do we want to do about the deinit check here:

// AsyncWriter.swift

  deinit {
    switch self._completionState {
    case .completed:
      ()
    case .incomplete, .pending:
      assertionFailure("writer has not completed is pending completion")
    }
  }

This will currently cause an assertionFailure if I add the following test to the integration tests, which shuts down the server before completing the RPC.

diff --git a/Tests/GRPCTests/AsyncAwaitSupport/AsyncIntegrationTests.swift b/Tests/GRPCTests/AsyncAwaitSupport/AsyncIntegrationTests.swift
index bc5f7eb4..298055cb 100644
--- a/Tests/GRPCTests/AsyncAwaitSupport/AsyncIntegrationTests.swift
+++ b/Tests/GRPCTests/AsyncAwaitSupport/AsyncIntegrationTests.swift
@@ -197,2 +197,11 @@ final class AsyncIntegrationTests: GRPCTestCase {
   }
+
+  func testServerCloseAfterMessage() {
+    XCTAsyncTest {
+      let update = self.echo.makeUpdateCall()
+      try await update.requestStream.send(.with { $0.text = "hello" })
+      _ = try await update.responses.first(where: { _ in true })
+      // server is closed in tearDown.
+    }
+  }
 }

Is it possible to re-jig the tests a little to explicitly close the server within the test? That way we can defer ending the request stream within the test?