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

chore: remove TWITTER_COOKIES env var #1288

Merged
merged 5 commits into from
Dec 21, 2024

Conversation

ChristopherTrimboli
Copy link
Contributor

Relates to:

No issue ticket, just a improvement, ease of setup change I noticed.

Risks

LOW - Twitter may not login? But tested with clearing cache and cached, working great.

Background

What does this PR do?

This PR removes the .env variable TWITTER_COOKIES and simply uses twitterScraper to manage cookies on login better from cache.

It also removes the docs / readme step where user has to provide the JSON array of TWITTER_COOKIES.

Side win: the looping try-catch while loop is cleaner to read, deleted many lines of code in simpler implementation for twitter auth.

What kind of change is this?

Improvements

Why are we doing this? Any context or related work?

TWITTER_COOKIES env var is an added complexity not required, users have to go searching in dev-tools console document.cookie and rearrange their JSON and add to ENV. But, twitter-scraper client has support for login + fetch current cookies on it's own. It is a redundant ENV setup that can be removed to simplify the twitter agent setup process for all users.

I recently implemented twitter-scapper-client in my own custom agent and never had todo this ENV setup, I saw it as a EZ win for Eliza community to port over.

Documentation changes needed?

If a docs change is needed: I have updated the documentation accordingly.

I have removed the TWITTER_COOKIES docs sections, full deletion, no longer needed.

Please show how you tested the PR. This will really help if the PR needs to be retested, and probably help the PR get merged quicker.

image

I also ran:

return await this.runtime.cacheManager.delete(
    `twitter/${username}/cookies`
);

To delete the cookies from cache manually to verify my code was working.

Testing

Where should a reviewer start?

Remove TWITTER_COOKIES from .env and see if Twitter agent boots correctly and works from fresh cache and already cached.

Detailed testing steps

None, automated tests are fine.

Deploy Notes

Remove TWITTER_COOKIES from any CI / CD and .env test runners.

Discord username

cjft

@ChristopherTrimboli ChristopherTrimboli changed the base branch from main to develop December 20, 2024 23:32
@odilitime odilitime changed the title remove TWITTER_COOKIES env var chore: remove TWITTER_COOKIES env var Dec 20, 2024
Copy link
Collaborator

@monilpat monilpat left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM - thanks for consolidating this on the client side :)

@monilpat monilpat merged commit 06e7527 into elizaOS:develop Dec 21, 2024
1 check passed
@touhonoob
Copy link

Getting DenyLoginSubtask with the new login method

 ["⛔ Failed to login to Twitter. Retrying... (0 attempts left)"] 

 ["⛔ Max retries reached. Exiting login process."] 

@adamcanray
Copy link

Getting DenyLoginSubtask with the new login method

 ["⛔ Failed to login to Twitter. Retrying... (0 attempts left)"] 

 ["⛔ Max retries reached. Exiting login process."] 

+1

@daniel-farina
Copy link

daniel-farina commented Jan 8, 2025

I think we should revert this as login with cookies is the only method that works currently for many users. On a fresh installation login with username/pw fails. Once I revert this PR and add cookies everything works.

cc: @monilpat @lalalune

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.

5 participants