Jessecar96 / SteamBot

Automated bot software for interacting with Steam Trade
http://scrap.tf
MIT License
1.33k stars 910 forks source link

If server is down when accepting trade offer, trade offer is not handled until bot restart #804

Open nuller1joe opened 9 years ago

nuller1joe commented 9 years ago

The case is pretty specific, but none the less quite important for Jessecar96 steambot.

I feel like its quite common that steam web api returns an internal error 500 when trying to accept a tradeoffer. I guess you cant really do anything about this. But what i noticed was that a tradeoffer which fails on accepting, will not be handled again before the bot restart.

I've been looking a little into the code, and as far as i can see the problem is cause by:

In TradeOfferManager method SendOfferToHandler(Offer offer), and the hashset: tradeOfferHistory.

So everytime a new tradeoffer is recieved, it is added to the tradeOfferHistory hashset. This is done before the we handle the tradeoffer, so in case the tradeoffer, does not go as planned (an exception is thrown), we will not revisit this specific offer even though we never finished it. (I dont know what the purpose of this hashset is?),

 private void SendOfferToHandler(Offer offer)
 {
        var tradeOffer = new TradeOffer(session, offer);
        tradeOfferHistory.Add(offer.TradeOfferId);
        OnNewTradeOffer(tradeOffer);
  }

If we look at both GetTradeOffersSince() and GetActiveTradeOffers() they have this statement:

 if (offer.TradeOfferState == TradeOfferState.TradeOfferStateActive && !tradeOfferHistory.Contains(offer.TradeOfferId))

Which shows us that an already "handled" offer will not be handled again. Im not 100% sure about this, but i wanted to share this with you, as it is causing me problems right now. I've not tried a fix for this yet. First i wanted to make sure that im correct about this, and it actually is this, that is causing the problem.

The fix though, would simply be to switch lines between tradeOfferHistory.Add(offer.TradeOfferId) and OnNewTradeOffer(tradeOffer)

RatexIndex commented 9 years ago

i test this atm but i think its only a misstake not a bug ^^

TheAnthonyNL commented 9 years ago

Well i can tell you that switching those lines will give this error:

ERROR: System.NullReferenceException: Object reference not set to an instance of an object.
   at SteamBot.DepositTradeOfferUserHandler.OnNewTradeOffer(TradeOffer offer)
   at SteamBot.Bot.TradeOfferRouter(TradeOffer offer)
   at SteamTrade.TradeOffer.TradeOfferManager.SendOfferToHandler(Offer offer) in c:\Users\A\Desktop\SteamBot-master\SteamTrade\TradeOffer\TradeOfferManager.cs:line 142
   at SteamTrade.TradeOffer.TradeOfferManager.GetTradeOffersSince(Int32 unixTimeStamp) in c:\Users\A\Desktop\SteamBot-master\SteamTrade\TradeOffer\TradeOfferManager.cs:line 87
   at SteamTrade.TradeOffer.TradeOfferManager.GetOffers() in c:\Users\A\Desktop\SteamBot-master\SteamTrade\TradeOffer\TradeOfferManager.cs:line 113
   at SteamBot.Bot.<HandleSteamMessage>b__10(NotificationCallback callback)
   at SteamKit2.CallbackMsgExtensions.Handle[T](ICallbackMsg msg, Action`1 handler)
   at SteamBot.Bot.HandleSteamMessage(ICallbackMsg msg)
   at SteamBot.Bot.BackgroundWorkerOnDoWork(Object sender, DoWorkEventArgs doWorkEventArgs)
nuller1joe commented 9 years ago

This does not make any sense to me. I dont think that NOT adding the offer to the hashset before handling the offer causes the offer to be null in the userhandler. I dont think "my" fix is causing this.

I dont get this error atleast

scholtzm commented 9 years ago

The reason for logging previous trade offers is simple - it allows you to ignore them completely. If SteamBot didn't do this, the same trade offer would appear over and over on every trade poll.

nuller1joe commented 9 years ago

But isnt the purpose of GetTradeOffersSince to do that? The first method to handle tradeoffers look like this:

 public bool GetOffers()
    {
        if (LastTimeCheckedOffers != 0)
        {
            bool action = GetTradeOffersSince(LastTimeCheckedOffers);

            if (action)
                LastTimeCheckedOffers = GetUnixTimeStamp();
            return action;
        }
        else
        {
            bool action = GetActiveTradeOffers();
            if (action)
                LastTimeCheckedOffers = GetUnixTimeStamp();
            return action;
        }
    }

I thought the purpose of all this was not to keep going around the same trade offers?

But i assume that there was some problems with this, since they added the hashmap. But never the less, i would still say that my fix fixes the problem, as the trade offer should only be marked as done when its actually done

scholtzm commented 9 years ago

Well, I suppose only @StormReaper knows why he added it. :grinning:

nuller1joe commented 9 years ago

Yup. There is one thing i can think off, but i dont know if its possible. Can a tradeoffer be corrupt? In this case, adding the offer to the hashmap before handling the offer makes perfect sense. But in any other case, i think its better to add the offer to the hashmap after you are doing handling it

bartico6 commented 9 years ago

No, tradeoffers cannot be corrupt. They can arrive to you corrupted due to your connection/valve servers sending malformed data. I'd validate the offer after sending the .Accept() method through. I'll talk about this more tomorrow though. School n' shit

bartico6 commented 9 years ago

Let me elaborate on the snipper posted above as it's fairly easy to understand. Upon startup, the bot has its LastTimeCheckedOffers timestamp set to zero, which is a (afaik) millis-since-1970 format. If it's zero, then the bot does a full scan of all ACTIVE offers. If it has a value other than zero, this means offers have been fetched at least once during this session, and it will instead get offers that arrived since the timestamp the offers were last fetched. As for the "offer unavail. for handling if Steam 500's while accepting" - that's up to YOU to take care of. It's a Steam bug, more than a Bot bug. If you press "Accept offer" on steam you would expect the offer to be accepted, but sometimes Steam says "fuck you" and errors out. You then retry accepting. Overall there are two solutions to this: One is to manually validate if the offer was actually changed by Steam's backend after you sent them a signal. Another is to modify the bot's core code to look out for that, and retry accepting/declining if no valid response is returned after requesting a change within the offer object. Which one is easier to implement - up to you :+1: