[suggestion] Not found offer in `getOffer` shouldn't return an Error.
sentisso opened this issue · 3 comments
Hello, IMO the getOffer
method shouldn't return an error in case the offer wasn't found.
Currently there isn't a rational way to detect, if an offer no longer exists. It is only achievable in a similar fashion:
manager.getOffer(offerId, (err, offer) => {
if (err && err.message === "No matching offer found") {
// trade offer wasn't found
...
} else if (offer) {
// trade offer was found
...
} else {
// we got an error, maybe retry later?
...
}
})
and I think this is not a good design, because of the error message comparison.
My proposal would be to call callback(null, null);
here.
Meaning "no error and no trade offer => the trade offer doesn't exist".
Note: I'm not sure how funny Steam servers are when it comes to returning accurate data about currently active trade offers.
So if it is possible that the !body.response.offer
condition is true
even though the trade offer does indeed exist and therefore can be retrieved later, then I would leave it as it is now.
Okay, nevermind, I think this is good design, because the method is suppose to return a specific trade offer.
If it isn't able to find the trade offer, then that's an error scenario => it should return an error.
Better solution to my problem would be to call the getOffers
method and filter out the original offerId
locally myself.
You don't need to check the error message, just check the eresult property on the Error object.
const TradeOfferManager = require('steam-tradeoffer-manager');
const {EResult} = TradeOfferManager;
let manager = new TradeOfferManager();
// ...
manager.getOffer(badOfferId, (err, result) => {
if (err) {
if (err.eresult == EResult.NoMatch) {
// the offer doesn't exist
} else {
console.log(`Something else happened, maybe we have an error code: ${EResult[err.eresult]}`);
}
return;
}
// the offer does exist
});