Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Engine-Twitter: set an error message when the user is not found #204

Open
wants to merge 1 commit into
base: stable
Choose a base branch
from

Conversation

knocte
Copy link
Contributor

@knocte knocte commented Jan 28, 2015

Set an error message when the user is not found, otherwise the
error message shown to the user as "Reason" would be the useless
"Exception of type 'Twitterizer.TwitterizerException' was thrown.",
when launching a command such as /timeline non-existing-username.

Set an error message when the user is not found, otherwise the
error message shown to the user as "Reason" would be the useless
"Exception of type 'Twitterizer.TwitterizerException' was thrown.",
when launching a command such as /timeline non-existing-username.
@knocte knocte force-pushed the twitter_user_not_found branch from cb0de6a to d26a357 Compare January 28, 2015 21:22
@meebey
Copy link
Owner

meebey commented Jan 28, 2015

I think this is a bug in Twitterizer that I need to fix. It should set the error message from Twitter but it doesn't, thus it fallbacks to the TypeName

@knocte
Copy link
Contributor Author

knocte commented Jan 28, 2015

@meebey, look at the whole if block:

            // HACK: Twitter returns HTML code saying they are overloaded o_O
            if (response.Result == RequestResult.Unknown &&
                response.ErrorMessage == null) {
                response.ErrorMessage = _("Twitter didn't send a valid response, they're probably overloaded");
            } else if (response.Result == RequestResult.FileNotFound &&
                response.ErrorMessage == null) {
                response.ErrorMessage = _("Twitter user not found");
            }

So it seems another case of ErrorMessage being null from the server.

@meebey
Copy link
Owner

meebey commented Jan 28, 2015

Oh yeah, that could be. But IIRC they don't return specific error codes but 200, else the HTML page would not render in browsers.... but this new mapping makes the error message of your case nicer.

@knocte
Copy link
Contributor Author

knocte commented Jan 28, 2015

Actually, just debugged Twitterizer lib, and the response message is (in line 248 of TwitterCommand class):

twitterResponse.Content = "{"errors":[{"message":"Sorry, that page does not exist","code":34}]}"

If one were to fix Twitterizer, the message is not much better than the one I used here!

@meebey
Copy link
Owner

meebey commented Jan 28, 2015

Sso there is a Twitterizer bug, but even if that was fixed it would present a useless error message so the command needs to provide the message itself

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants