Version 2 wishlist

[madsmtm]

I've been brewing ideas for v2 for a while now (probably over a year), and I started implementing some of these ideas in a separate branch (completely rewriting the code). Inevitably, the master branch kept getting worked on, and it eventually fell apart.

So now I'm trying to reboot some of my ideas, but I'll take a separate approach to implementing them: I'll implement almost everything that can be done without making breaking changes, and then I'll move the project into a state of v2, where we can properly progress towards a cleaner API. In general, I'd much rather have a stable, minimal API, than an unstable, comprehensive API!

I've tried to assemble a list of major things I'd like to see before making breaking changes:

• [x] Use flit as our build system (see #382)
• [x] Use black for formatting (see #386)
• [x] Refactor code into logical, separate files (client.py is far too large)
  • [x] Would help us to make proper unit tests
• [ ] ~Set up integration tests using something like VCR.py~
• [x] Clean up documentation
  • [x] Fix setup (tracked in #391)
  • [ ] Make @carpedm20 set up the webhook, so that tag creation / deletion is tracked as well
  • [x] API rundown (Use Google Docstring Style)
  • [x] Introduction overview overhaul
  • [x] Investigate tools for detecting docstring errors and spelling mistakes
• [x] Inline examples
• [x] More detailed examples (recommends good practice too)
• [ ] API coherence (fetchThreadMessages can't fetch "system" messages / events)

And a list of major things I'd like to change:

• [x] Drop Python 2 support (It'll probably reach it's EOL before we get everything done 😁)
• [x] Use type hints
  • [x] Improve documentation with sphinx-autodoc-typehints
• [ ] Consider async options
  • [ ] See sphinxcontrib-trio for documentation improvements on this.
• [x] Declare a proper public API. See trio as a reference to a project that fullfulls this promise (e.g. using _ before methods and filenames)
• [ ] Declare a concise API (e.g. document expected exceptions)
  • [x] Using datetimes, see #278, is a subtask of this
• [x] Use snake_cased method names like PEP8 suggests
• [x] Fix logging (see #258)
• [x] Make a distinction between models, like Message, and what you can send, as parameters in send. Though combining these sound good idea in theory, it's really messy in practice, and not really that useful.
• [x] Make a distinction between user created models and recieved models
  • [x] Building upon this idea, we can move a lot of implementations into the model instead of a central client. For example, I'd like to be able to do something like:

    message = Message(id="123", client=client)  # Does not make an external call (since you don't need to, to interact with the message)
    message.react(None)
    assert not hasattr(message, "text")
    # Or alternatively:
    message = client.get_message("123")  # Makes an external call, and returns `MessageData`, a subclass of `Message`
    message.react("👍")
    assert hasattr(message, "text")
    

But anyway, it's late, and I'm rambling, and a lot of the later bullet points are still pretty abstract ideas. We should focus on the most pressing points first 😁.

If you have something you'd like to put on the wishlist, or would like to point the project in a different direction, suggest away, I'd love to hear what you have to say! 😊

[darylkell]

As I've been building a tool to capture conversation threads over the last few weeks, I've noticed these things:

• The models docs for LiveLocationAttachment objects don't accurately represent all the attributes that the object has (the docs declare only three - expiration_time, is_expired, and name - but it looks like a LiveLocationAttachment also has the same attributes that a LocationAttachment object has). Once expired, doesn't have any other attributes/information like web Messenger presents (namely text, being the last updated time).

• System messages like "Person has changed the name of the conversation to 'X'" or "Person has left the group" don't have that text anywhere in the message - essentially it only returns message.author for the person involved and blank everything else. I built out a list of thread participants by using a thread's 'participants' attribute (ie those still in the thread), plus all the message.author's with blank messages. I just can't reproduce those system messages within the conversation.

• Polls don't appear to have any representation in messages, similar to system messages mentioned above (has message.author but no other attributes specific to the poll).

• Games don't have any representation in messages, similar to polls and system messages mentioned above.

A lot of hard work done here, I'm very appreciative of it all.

*** edit to add other points ** edit: Is it possible to apply the listen method client._parseMessage() logic to add some of the features lacking in client.fetchMessageInfo()? (mentioned above - polls, games, added/left participants, calls, other system messages etc)

[madsmtm]

Thanks for the feedback, good to hear the library has been useful to you 😊!

• Regarding LiveLocationAttachment, then that's because it inherits LocationAttachment, see models.py, line 467
• Hmm, you have a point about system messages, they're currently only retrievable using Client.listen and Client.onX events, and not with Client.fetchThreadMessages. Did you get the empty Messages by using fetchThreadMessages? Because if so, then that's a bug 😉
• Again, polls are only really implemented while listening, and the poll messages would classify as system messages, but we should be able to fetch which poll we have in a thread
• Nope, I tried messing with the API, but it was awfully complicated. We might be able to parse game events though (and not necessarily be able to execute them ourselves)

And no, the way Client._parseMessage is built, you can't really extend it yourself, but I do plan on changing that!

Anyway, the general pattern I'm noticing from what you're saying, is a lack of stability and cohesiveness, (a lot of small bugs, and incomplete functionality) which I'm irritated about myself. So those parts are definitively going to change! 👍

[darylkell]

• I figured it was inheritance but hadn't checked it out beyond the assumption, just noted the documentation didn't touch on it.
• Yeah, the empty messages were from client.fetchThreadMessages but at least it was helpful in building the list of thread participants beyond only those that were currently in thread.
• The stuff client._parseMessage handles while listening would really tie a bow in things if it can be implemented while fetching thread messages!

I've found the repo to be awesome! I just noticed a few things, and thought to mention them here when I saw this issue. And you guys have been awesome in helping a GitHub newbie out. From what I have experienced from the repo, it's mainly the missing functionality that makes a difference - but it is oh so close to having everything! From the point of view of someone who is trying to use an API to effectively capture a conversation thread in its entirety.

[madsmtm]

I hear what you're saying, and I'll try to find time to add the missing functionality to fetchThreadMessages! Again, thanks for your valuable input! 👍

[szymonszl]

I'd definitely add some User object caching support (like mentioned in issue #320), at least for names.

[madsmtm]

Huh, totally forgot that thread, but yeah, definitely, thanks for the tip!

[joakimed]

In the name of user friendliness, I would like to request an example of how best to handle background listening. My current implementation inspired by the echobot example locks up the entire program.

After a bit of searching, I have discovered the threading and subprocess modules. Both of these seem daunting, overkill, and not very good options for what is hopefully going to be a multi-plattform application.

I realize this is most likely related to my lack of experience, and using the wrong language for the task. But isn't that what user friendliness is all about? 😀

[madsmtm]

Doing something in the background is a problem in python in general. The nice solution would be to use the new async/await syntax, but sadly, the python ecosystem is far from mature regarding async networking. We could use asyncio/aiohttp, but that means that we have to fully build our system around their infrastructure. What we really need the push for urllib3 to add async support, see this tracking issue: https://github.com/urllib3/urllib3/issues/1323. That would mean we'd be able to support a variety of async backends (and not just asyncio), while also supporting syncronous code!

Sadly, this is pretty far off into the future, so your best bet is the threading module, but since the underlying library we're using (requests) is not thread safe, you should watch out! E.g. I don't think it's safe to run the listener loop while sending a message in another thread. I can't really give you any useful information on how to do this, I haven't tried it myself (as you said, it's quite "daunting" 😁). But thanks for your input though, perhaps it'll be possible to make a few helper functions for spawning the listener in a separate thread? Or as you said, improve the examples? I'll make sure to take a look! 👍

[joakimed]

It turns out it was not as big of a problem as I anticipated. I've been playing around with this for a couple of hours and here is what I came up with. It's painfully inefficient, but it works (ish). If this was being used in a gui application, most of the problems would solve themselves.

Thanks for your help!

[madsmtm]

Thanks for the example! 👍 If you want, you can make a PR to put it into the examples folder? Otherwise, I'll probably do it myself at some point 😉

[joakimed]

I would if i could get the last line working. :see_no_evil: Not sure if this is related to the threading, or if I'm missing something related to fbchat. I pretty much copied the subclass from EchoBot. Either way it doesn't seem like the best example until that gets sorted.

[kapi2289]

@joakimed There's one problem... you've created a new class, but... you don't use it anywhere! You must do

client = PrintMessage(email, pw)

instead of

client = Client(email, pw)

[szymonszl]

Wouldn't it be simpler (and considering @madsmtm 's comment, propably also safer) to use an external event loop instead of playing with threading? IIRC the listen function is rather simple and should be not that hard to remake in a different eventloop. It should be possible to make your own while True which both calls doOneListen and whatever else you want, like checking for user input. When I'm able to I'll try to check if it's possible by integrating fbchat into, maybe tkinter? Let's see.

Now that I think of it, it also might be possible to add some kind of onOneListen handler function to instead be able to use fbchat's loop as the program's main event loop.

PS. always store/use session cookies if you can, lots of consecutive logins can easily make your account flagged

[joakimed]

@kapi2289 Ofcourse, so simple.

@szymonszl Thanks for the valuable pointers. I'm going to play around some more with this, focusing on making it safer. Hopefully with a single login request. As for session cookies, I intentionally avoided them in this script as they are not used in the other examples.

This is getting way off topic. It was never my intention to hijack this issue, so if i run into more problems I'll open a new issue.