Add Custom Errors #85
Conversation
|
When resolving the conflict, feel free to just accept all of your changes, it came from updating the |
…wa-main # Conflicts: # src/hiscores.ts
src/hiscores.ts
Outdated
| ): Promise<HiscoresResponse | undefined> => | ||
| otherGamemodes.includes(mode) | ||
| ? getOfficialStats(rsn, mode, options?.axiosConfigs?.[mode]) | ||
| .catch(err => err) |
There was a problem hiding this comment.
Similarly to the comment for the emptyResponse, we need to return undefined in the case of a caught error. Otherwise the conditions of the if statements below will be true when the error object is present as the response.
We should probably log the error in some way, or maybe even consider returning the errors in some format. But on the other hand I'm also hesitant to modify the structure of the Player object. 🤔 Not entirely sure what to do here, but since we're updating errors in the PR, it seems like a good opportunity to improve error handling in general.
There was a problem hiding this comment.
Good point. I wonder what the expected behavior would be for the case where the initial stat lookup succeeds, but there is a 500 status code during the subsequent lookups. We could add an optional strict parameter that throws if any of the lookups error.
I will check what the previous behavior would have been prior to the JSON endpoint update. Realistically, the only error that would occur is HiScoresError, unless the account in question name changes or gets banned in-between the first lookup and one of the gamemode lookups.
There was a problem hiding this comment.
I like the idea of a strict mode! I think that if getStats is used for a main account, it will 404 when looking up ironman stats. So we'd probably want to restrict it to only throwing/reporting on HiScoresErrors. But I think it's fair to wait for a future enhancement to implement this so we don't delay this great change any longer!
|
Thank you for this change and the quick modifications despite my lengthy delays! 😅 This is a nice improvement and will hopefully alleviate your rate-limiting issue. I will be publishing this shortly. |
|
Happy to contribute to something that I have been using for so long. Thanks for the insightful feedback on the changes! Looking forward to the next opportunity to work with you on this. |
closes #84
InvalidFormatError,InvalidRSNError,PlayerNotFoundError, andHiScoresErrorgetStatsto handle errors from Axios