SteamKit icon indicating copy to clipboard operation
SteamKit copied to clipboard

Handle k_EMsgDestJobFailed

Open Netshroud opened this issue 8 years ago • 3 comments

Proof of concept:

        class JobFailureHandler : ClientMsgHandler
        {
            public override void HandleMsg(IPacketMsg packetMsg)
            {
                if (packetMsg.MsgType != EMsg.DestJobFailed)
                {
                    return;
                }

                var failure = new ClientMsg<MsgClientJustStrings>(packetMsg);
                var callback = new DestinationJobFailedCallback(failure.TargetJobID);
                steamClient.PostCallback(callback);

                Console.WriteLine("Job failed!");
            }
        }

        public class DestinationJobFailedCallback : CallbackMsg
        {
            public DestinationJobFailedCallback(JobID jobID)
            {
                this.JobID = jobID;
            }
        }

Only using MsgClientJustStrings as a placeholder. Actual response seems to have a 1-byte body, so far I've only seen it set to 0x00.

Not sure how this would work with #170.

Netshroud avatar Oct 18 '15 07:10 Netshroud

So the behavior I'm seeing in steamclient regarding DestJobFailed is in CJob::BYieldingWaitForMsg. Here's a rough decompilation:

bool CJob::BYieldingWaitForMsg( IMsgNetPacket **ppNetPacket )
{
  JobMsgInfo_t *pJobMsgInfo; // [sp+18h] [bp-10h]@5

  if ( !this->m_szYieldReason )
  {
    SetYieldInfo( "Wait for msg", NULL );
  }

  if ( GetJobMgr().BYieldingWaitForMsg( this, ppNetPacket, &pJobMsgInfo ) )
  {
    while ( pJobMsgInfo->m_EMsg == k_EMsgJobHeartbeat )
    {
      m_STimeNextHeartbeat.SetFromServerTime( k_cMicroSecJobHeartbeat );
      SetYieldInfo( "Wait for msg", NULL );

      if ( !GetJobMgr().BYieldingWaitForMsg( this, ppNetPacket, &pJobMsgInfo) )
        return false;
    }

    AddPacketToList( ppNetPacket, pJobMsgInfo->m_JobIDSource );

    return pJobMsgInfo->m_EMsg != k_EMsgDestJobFailed;
  }

  return false;
}

Essentially, JobHeartbeat will keep a job alive on the client (by extending its timeout), and the final success of the job comes down to the EMsg the server sends down. The body of the message isn't inspected for heartbeats or failures.

This doesn't really translate well to callbacks, but could probably be handled gracefully with the existing work in #170.

voided avatar Oct 19 '15 02:10 voided

Oh, so that's where it is.

Some Jobs also use this one (Hopper pseudocode here), which seems more appropriate for our usage:

int CJob::BYieldingWaitForMsg(IProtoBufMsg*, EMsg)(void * arg_0, void arg_4) {
    var_10 = *___stack_chk_guard;
    eax = GetMessageList();
    CMessageList::GetMessage(eax, arg_8, var_214, 0x3b);
    CJob::SetYieldInfo(arg_0, var_214);
    if (CJob::BYieldingWaitForMsg(arg_0) != 0x0) {
            ebx = arg_4;
            eax = *ebx;
            (*(eax + 0x28))(ebx, 0x0);
            eax = *ebx;
            eax = (*(eax + 0x2c))(ebx);
            ecx = 0x1;
            esi = ___stack_chk_guard;
            if (eax != arg_8) {
                    eax = *ebx;
                    eax = (*(eax + 0x2c))(ebx);
                    CDbgFmtMsg::CDbgFmtMsg(var_210, "CJob::BYieldingWaitForMsg expected msg %u but received %u", arg_8, eax);
                    CDbgFmtMsg::CDbgFmtMsg(var_110, "Assertion Failed: %s", var_210);
                    AssertMsgImplementation(var_110, 0x0, "/Users/buildbot/buildslave/steam_rel_client_osx/build/src/common/job.cpp", 0x28e, 0x0);
                    ecx = 0x0;
            }
    }
    else {
            ecx = 0x0;
            esi = ___stack_chk_guard;
    }
    if (*esi == var_10) {
            eax = ecx & 0xff;
    }
    else {
            eax = __stack_chk_fail();
    }
    return eax;
}

Netshroud avatar Oct 19 '15 06:10 Netshroud

This is now handled for async jobs, but not for callback subscriptions.

I'm not sure what's the best way to handle this. The best I can think of is to use the same pattern used by Steamworks, where you can subscribe to a callback with Action<TCallback callback, bool didItFail>.

The other option I can think of is to get consumers to use async jobs instead of callbacks.

yaakov-h avatar Sep 03 '17 15:09 yaakov-h