pdinsdale / BattleBot

Discord.js bot for the 1-Up World Discord server
https://discord.gg/mario
5 stars 3 forks source link

Incorrect parameter usage for anonymous function #14

Closed sdace closed 5 years ago

sdace commented 5 years ago

https://github.com/Phoenix1128/BattleBot/blob/0e8036a213d047f0370e6fa6fd571998215a233a/commands/factions/clear.js#L37

If an error is caught, the parameter passed in the anon function is not the same as the then block. It will simply return an error that you're meant to print to the console for description of what went wrong. collection.size will always be null; you should instead be printing it directly, and assume that the entire command failed.

pdinsdale commented 5 years ago

It actually hasn’t been null from my tests but yes I’ll try to fix that up

pdinsdale commented 5 years ago

Looking back at this, it is the same parameter as the .then block so I’m not sure what you mean here

sdace commented 5 years ago

Perhaps it works sometimes; however if you look at the documentation, https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Promise/catch it's clear that you can't rely on this. There are several reasons why it might be rejected, it's not really a good idea to assume any one reason.

pdinsdale commented 5 years ago

Ah I understand, it needs to be an instance of Error instead of what I have. Yes, I know this now but this is old code where I relied heavily on examples, and this is something I took from said examples. I don’t do this anymore but I will remember to replace that when I get to it in the rewrite. Thanks!

pdinsdale commented 5 years ago

I ended up completely getting rid of this command in the rewrite since it was redundant to the new faction battle setup. I have made sure that all of my .catch statements catch an instance of Error though.