Error handling

[MarcoForlivesi]

If an error happens the plugin doesn't provide any event

[savasozer]

You can use FGrpcResult.Result.Code in response lambda to check occurred an error

[MarcoForlivesi]

@savasozer in case of error, the lambda function is not reached.

template<typename T, typename R>
class GrpcContext_Ping_Stream : public TGrpcContext<T, R>
{
	typedef TGrpcContext<T, R> Super;
protected:
	void OnRpcEventInternal(bool Ok, const void* EventTag, typename Super::FRpcCallbackFunc RpcCallbackFunc)
	{
		if (!Ok)
		{
			Super::RpcReaderWriter->Finish(&(Super::RpcStatus), Super::ReadTag);
			Super::UpdateState(EGrpcContextState::Done);
			return;
		}

As a test I deleted the authorization token from my service, in case of error it seems to go through here (TurboLinkGrpcContext.h), but the callback is not called and for some reason Super::RpcStatus turns out to be ok

The rpc method is:
rpc FromTextToVoice (TextData) returns (stream ResponseData);

[thejinchao]

Sorry, I'm very busy with work recently and don't have time to take care of this project. I will take a look at this issue when I have time.

[6r0m]

@thejinchao @MarcoForlivesi

I think I realized how it should work in Turbolink context)

1. when Ok=false we shouldn't call Super::UpdateState(EGrpcContextState::Done) in the OnRpcEventInternal (TurboLinkGrpcContext.h). because we need to wait when Super::RpcReaderWriter->Finish will be processed (another AsyncNext in the TurboLinkGrpcManager.cpp).
   only after next processing, proper status will be written in relevant context.

also we need to implement another tag for filtering the Finish call

if (!Ok)
{
	Super::RpcReaderWriter->Finish(&(Super::RpcStatus), Super::FinishTag);

	// Don't Update State to Done here, because need to wait when this operation finished on the next AsyncNext which updates Super::RpcStatus
	// Super::UpdateState(EGrpcContextState::Done);
	return;
}

2. on the next AsyncNext you will receive updated grpcContext with relative RpcStatus with code_ and error_message_

3. finally, in the OnRpcEventInternal (which will be called after AsyncNext in the relative context):
   we need to filter FinishTag after updating status and in case error push callback with wrong status

// Finish operation processed - handle error if exists and Update State to Done
if (EventTag == Super::FinishTag)
{			
	if (RpcCallbackFunc && !Super::RpcStatus.ok())
	{
		UE_LOG(LogTurboLink, Error, TEXT("CallRpcError: %s"), *result.GetMessageString());

		if (RpcCallbackFunc)
		{
			RpcCallbackFunc(result, nullptr);
		}
	}

	Super::UpdateState(EGrpcContextState::Done);
	return;
}

also, in case if we sure that receiving wrong status only possible after AsyncNext isn't Ok then could omit further else branch ```if (Super::RpcStatus.ok())``

slightly modified GrpcContext_Ping_Stream example:

template<typename T, typename R>
class GrpcContext_Ping_Stream : public TGrpcContext<T, R>
{
	typedef TGrpcContext<T, R> Super;
protected:
	void OnRpcEventInternal(bool Ok, const void* EventTag, typename Super::FRpcCallbackFunc RpcCallbackFunc)
	{
		if (!Ok)
		{
			Super::RpcReaderWriter->Finish(&(Super::RpcStatus), Super::FinishTag);

			// Don't Update State to Done here, because need to wait when this operation finished on the next AsyncNext which updates Super::RpcStatus
			// Super::UpdateState(EGrpcContextState::Done);
			return;
		}

		FGrpcResult result = GrpcContext::MakeGrpcResult(Super::RpcStatus);

		// Finish operation processed - handle error if exists and Update State to Done
		if (EventTag == Super::FinishTag)
		{			
			if (RpcCallbackFunc && !Super::RpcStatus.ok())
			{
				UE_LOG(LogTurboLink, Error, TEXT("CallRpcError: %s"), *result.GetMessageString());

				if (RpcCallbackFunc)
				{
					RpcCallbackFunc(result, nullptr);
				}
			}

			Super::UpdateState(EGrpcContextState::Done);
			return;
		}

		// In case if we sure that receiving wrong status only possible after AsyncNext isn't Ok then could omit further else branch
		if (Super::RpcStatus.ok())
		{
			if (Super::GetState() == EGrpcContextState::Initialing)
			{
				check(EventTag == Super::InitialTag);

				Super::RpcReaderWriter->Read(&(Super::RpcResponse), Super::ReadTag);
				Super::UpdateState(EGrpcContextState::Busy);
			}
			else
			{
				if (RpcCallbackFunc)
				{
					RpcCallbackFunc(result, &(Super::RpcResponse));
				}
				Super::RpcReaderWriter->Read(&(Super::RpcResponse), Super::ReadTag);
			}
		}
		else
		{
			UE_LOG(LogTurboLink, Error, TEXT("CallRpcError: %s"), *result.GetMessageString());

			if (RpcCallbackFunc)
			{
				RpcCallbackFunc(result, nullptr);
			}
			Super::UpdateState(EGrpcContextState::Done);
			return;
		}
	}
public:
	GrpcContext_Ping_Stream(FGrpcContextHandle _Handle, UGrpcService* _Service, UGrpcClient* _Client)
		: Super(_Handle, _Service, _Client)
	{
	}
};

thanks again for the plugin - it's a lifesaver!

[MarcoForlivesi]

Yes, it seems works, but you miss the part of the tag handling

    //async tag
    void* InitialTag = nullptr;
    void* WriteTag = nullptr;
    void* ReadTag = nullptr;
    void* FinishTag = nullptr;

void GrpcContext::UpdateState(EGrpcContextState NewState)
{
	if (GetState() == NewState) return;

	ContextState = NewState;
	if (ContextState == EGrpcContextState::Initialing)
	{
		InitialTag = Service->TurboLinkManager->GetNextTag(AsShared());
		WriteTag = Service->TurboLinkManager->GetNextTag(AsShared());
		ReadTag = Service->TurboLinkManager->GetNextTag(AsShared());
		FinishTag = Service->TurboLinkManager->GetNextTag(AsShared());
	}
	else if (ContextState == EGrpcContextState::Done)
	{
		Service->TurboLinkManager->RemoveTag(InitialTag);
		Service->TurboLinkManager->RemoveTag(WriteTag);
		Service->TurboLinkManager->RemoveTag(ReadTag);
		Service->TurboLinkManager->RemoveTag(FinishTag);
	}

	if (Client->OnContextStateChange.IsBound())
	{
		Client->OnContextStateChange.Broadcast(Handle, NewState);
	}
}

[6r0m]

yeap, you are right, I forget to note this. mostly to emphasize the async behavior.

also in this case, need to add logic above to another context behaviors

[MarcoForlivesi]

@thejinchao is it possible to integrate the changes proposed by 6r0m?

[bmurray]

I was beating my head against a wall trying to figure out what I was doing wrong, only to spend hours going down this rabbit hole. I don't know C++ GRPC very well, but this does seem like the best solution. Is someone working on a PR? That's probably the easiest way to get this fixed. I have another couple of bugs to report too, but those have (sort of) easy workaround. This doesn't.