linvi / tweetinvi

Tweetinvi, an intuitive Twitter C# library for the REST and Stream API. It supports .NET, .NETCore, UAP (Xamarin)...
MIT License
1k stars 220 forks source link

Unable to search with geocode in 0.9.12.2 #237

Closed JoshKeegan closed 8 years ago

JoshKeegan commented 8 years ago

Hi Linvi, Bit of a strange one, but looks very similar to #221.

After updating to 0.9.12.2, when doing a search against the REST API, a simple search like "test" works fine but I've noticed one with a geocode, e.g. "geocode:51.5287718,-0.24168,50km" doesn't. Error message:

--- Date : 27/05/2016 15:45:22
URL : https://api.twitter.com/1.1/search/tweets.json?q=geocode%3a51.5287718%2c-0.24168%2c1km&result_type=recent&count=100
Code : 401
Error documentation description : Unauthorized -  Authentication credentials were missing or incorrect.
Error message : https://api.twitter.com/1.1/search/tweets.json?q=geocode%3a51.5287718%2c-0.24168%2c1km&result_type=recent&count=100 web request failed.
Could not authenticate you. (32)

Since the error is almost identical to the one in #221, this is probably already fixed but just wanted to check it's working in 0.9.13.0? Cheers, Josh

linvi commented 8 years ago

It does work properly for me. Here is the code I tried :

var tweets = Search.SearchTweets(new TweetSearchParameters("hello")
{
    GeoCode = new GeoCode(-0.24168, 51.5287718, 50, DistanceMeasure.Kilometers)
});
JoshKeegan commented 8 years ago

This geocode was entered in the search text, not passed into the search parameters. Does it work for you in the text? Something like (untested):

var tweets = Search.SearchTweets(new TweetSearchParameters("geocode:51.5287718,-0.24168,50km"));
linvi commented 8 years ago

This syntax is not supported but I guess I can add this as a feature. In addition the syntax would use an = instead of :.

linvi commented 8 years ago

So I think I could support text formatted query. But this will have to be a new feature. In the meantime the code that you gave do not throw in version 0.9.13.0.

JoshKeegan commented 8 years ago

This isn't really something you should have to support and is something we've relied on for some time without issue as Twitter supports operators being placed in the text supplied to the "q" parameter (which are in the form "operator:value", see https://dev.twitter.com/rest/public/search).

So as long as the text supplied to the q parameter is escaped correctly and not otherwise intefered with before being sent to Twitter, I don't see what would need supporting from Tweetinvi?

linvi commented 8 years ago

Twitter supports operators being placed in the text supplied to the "q" parameter

I did not remember about that :s

So as long as the text supplied to the q parameter is escaped correctly and not otherwise intefered with before being sent to Twitter, I don't see what would need supporting from Tweetinvi?

You are correct. I just thought I would have to do the parsing of the query to supply the parameters.

The problem cannot be reproduced on 0.9.13.0. Are you happy for me to close it?

JoshKeegan commented 8 years ago

Yeah, it can be closed. I'll assume any more errors I get related to missing credentials are the same thing as these last two. Thanks :)

linvi commented 8 years ago

I think it is related with a fix that I did on the search query builder.

linvi commented 8 years ago

Please keep reporting them even if you think they share the same root problem. I prefer to make sure that it is actually fixed instead of just assuming so.

linvi commented 8 years ago

Would you want an alpha of 0.9.13.0?

It would not include the RateLimits improvements.

JoshKeegan commented 8 years ago

Fair enough, will do.

I'm in no rush to upgrade from 0.9.10.2 but would happily run an alpha of 0.9.13.0 (not in production) if you wanted me to. I currently track RateLimits outside of Tweetinvi, so wouldn't be using any improvements to RateLimits anyway so that's not an issue.

linvi commented 8 years ago

I will upload an alpha tonight.

Just out of curiosity. How are you tracking your RateLimits? We are in the process of improving the precision of the RateLimits. Because you are not using it, you are a perfect candidate to ask some feedback.

What do you think should be improved so that you use Tweetinvi RateLimits?

I personally think there are few features missing from Tweetinvi and I myself rely on custom code in parallel of RateLimitTrackerOptions.TrackOnly;.

Your feedback could be quite precious :)

linvi commented 8 years ago

If you have some feedback to share it would be good if you do it here https://github.com/linvi/tweetinvi/issues/218.

JoshKeegan commented 8 years ago

I won't muddy up #218 with this as I'm not familiar with the current state of Tweetinvi's rate limit maganement, so this isn't really feedback. If you think my use case is something that could now be done by Tweetinvi let me know & I could perhaps look into it as an option and then give decent feedback.

It's been a very long time since I looked at the rate limits tracking for either Tweetinvi or my own code, so take all of this with a pinch of salt. It may no longer be a good idea for my tracking to be completely independant of Tweetinvi's, even if I didn't rely completely on Tweetinvi for their management.

The aim for some parts of my system is to poll Twitter as a data source and keep our own database up to date. So the aims are:

Additional implementation requirements:

Another advantage to tracking rate limits myself, but not something I've actually done was the thought that it would be easier to distribute the Poller application over multiple servers at a later date. Whilst I'm sure this could be done with Tweetinvi's tracking too, having an intimate understanding of whatever Rate limits tracking method you're using would likely make it quicker to distribute.

I think that those basic aims would be best achieved with some additional code on top of Tweetinvi's own rate limits tracking, but at the time there were problems with doing this. I can't remember the specifics, but I think one of them might have been not counting multiple API requests if Tweetinvi had to make 2+ requests internally to fulfill your request.

The additional implementation requirements confuse things slightly because the our own rate tracking has been bullet proof here, so I have no desire to replace it in the short-term. Long-term it's an option as it would be cleaner to re-use anything out of Tweetinvi and the current system isn't perfect by any means!

JoshKeegan commented 8 years ago

Oh, and don't rush for an alpha tonight. I won't be working over the bank holiday weekend so Tuesday would be the earliest I'd look at it :)

Have a good weekend!

linvi commented 8 years ago

Here you go. Latest version including the RateLimits improvements.

Tweetinvi 0.9.13.0 - Binaries.zip

JoshKeegan commented 8 years ago

I've been running the attached alpha for 0.9.13.0 since Tuesday & haven't noticed any problems yet, so all looks good :)

linvi commented 8 years ago

I am glad to hear it. I hope the release version will work as smoothly as the alpha. I am doing loads of changes right now!