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.

Sign up for GitHub

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

Jump to bottom

fix: Client.logout #2661

Open
wants to merge 24 commits into
base: main
Choose a base branch
from
Open

fix: Client.logout #2661

wants to merge 24 commits into from
+6 −9

Conversation

alechkos
Copy link
Collaborator

@alechkos alechkos commented Dec 2, 2023 •
edited

Important

breaking change Fix or feature that would cause existing functionality to change

Table of Contents

- Description

- Related Issues

- I Want to Test this PR

- I Got an Error While Testing This PR ❌

- How Has the PR Been Tested (latest test on 06.12.2023)

- Types of Changes

Description

The Client.logout method has executed the WA logout function and straight after that closed the browser with the deletion of the session folder.
This resulted in the issue that the current session on the user's phone did not have sufficient time to complete and remained hanging in the list of sessions.
Additionally, the Client's disconnected event was not being fired as expected.

The PR fixes that issue by subscribing to the Puppeteer disconnected event withing the Client.logout method.
Now when logout is called, the session on the linked device is terminated first.
This triggers the Client's disconnected event, which then closes a browser and fires the Puppeteer disconnected event ultimately removing the session folder.

❗ Breaking Change:

If a current session is terminated from the user's phone, the session folder will be removed, as it is supposed to be

Related Issues

The PR closes #2360, closes #2354, closes #2110, closes #2303, closes #2581, closes #2746

To test this PR by yourself you can run one of the following commands:

  • NPM
npm install github:alechkos/whatsapp-web.js#fix-logout
  • YARN
yarn add github:alechkos/whatsapp-web.js#fix-logout

If you encounter any errors while testing this PR, please provide in a comment:

  1. The code you've used without any sensitive information (use syntax highlighting for more readability)
  2. The error you got
  3. The library version
  4. The WWeb version: console.log(await client.getWWebVersion());
  5. The browser (Chrome/Chromium)

Important

You have to reapply the PR each time it is changed (new commits were pushed since your last application)

How Has The PR Been Tested (latest test on 06.12.2023)

  1. First case:

    1. Create a WWeb sesion using LocalAuth authstrategy
    2. Wait until the data is synchronized between the WWeb and the linked phone
    3. Call the function: 
      // ...
await client.logout(); // => The same linked session on the device is terminated as expected
// ...
    4. The function will fire the Client's disconnected event since the linked session on the device was terminated: 
      client.on('disconnected', (reason) => {
    console.log(reason); // => 'NAVIGATION'
});
    5. Ensure that the linked session on the device was terminated successfully and the session folder is removed

  2. Second case:

    1. Create a WWeb sesion using LocalAuth authstrategy
    2. Wait until the data is synchronized between the WWeb and the linked phone
    3. Terminate the current linked session on the phone (via 'Log out' button on the phone)
    4. The disconnected event will be fired since the linked session on the device was terminated: 
      client.on('disconnected', (reason) => {
    console.log(reason); // => 'NAVIGATION'
});
    5. Ensure that the session folder is removed successfully as expected

Tested On:

Types of accounts:

  • Personal
  • Buisness

Environment:

  • Android 10:
    • WhatsApp: latest
    • WA Business: latest
  • Windows 10:
    • WWebJS: 1.23.1-alpha.0
    • WWeb: v2.2353.0
    • Puppeteer: v18.2.1
    • Node: v18.17.1
    • Chrome: latest

Types of Changes

  • Dependency change
  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix that would cause existing functionality to change)

Checklist

  • My code follows the code style of this project
  • I have updated the usage example accordingly (example.js)
  • I have updated the documentation accordingly (index.d.ts)

@alechkos
fix: Client.logout removes session properly
31de5d4 
While loging out the session in WWeb, the method also removes the session
from all linked devices leaving no 'tails'
@alechkos
fix: typo
0850117
@alechkos alechkos marked this pull request as draft December 3, 2023 03:56
@alechkos alechkos marked this pull request as ready for review December 3, 2023 22:02
@jacopobonomi
Copy link

Thanks for all!

@alechkos alechkos added the breaking change Fix or feature that would cause existing functionality to change label Dec 7, 2023
shirser121
shirser121 reviewed Dec 17, 2023
View reviewed changes
Copy link
Collaborator

@shirser121 shirser121 left a comment

Choose a reason for hiding this comment

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

You cant to logout after destroy or close the page

@alechkos
Copy link
Collaborator Author

@shirser121

You cant to logout after destroy or close the page

authStrategy.logout just deletes the session folder, so you can do it after destroying a page, also i have tested the pr

Davixe7
Davixe7 approved these changes Jan 6, 2024
View reviewed changes
Copy link

@Davixe7 Davixe7 left a comment

Choose a reason for hiding this comment

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

Works as expected in both cases:

  1. Session list item is removed after calling: await Client.logout()
  2. "Disconnected" event is fired after closing the session via phone.

@alechkos alechkos mentioned this pull request Jan 14, 2024
Closed
1 task
@alechkos alechkos mentioned this pull request Jan 19, 2024
Open
1 task
@ekapasha17
Copy link

why do I need to wait for one minute, I've tried to disconnect under one minute and I get similar error like this :
ProtocolError: Protocol error (Network.getResponseBody): Target closed

@alechkos
Copy link
Collaborator Author

@ekapasha17

why do I need to wait for one minute, I've tried to disconnect under one minute and I get similar error like this : ProtocolError: Protocol error (Network.getResponseBody): Target closed

Who told you you have to wait 1 minute?

@ekapasha17
Copy link

@ekapasha17

why do I need to wait for one minute, I've tried to disconnect under one minute and I get similar error like this : ProtocolError: Protocol error (Network.getResponseBody): Target closed

Who told you you have to wait 1 minute?

nobody told me, but based on my experience when I disconnect under one minute I get the error but when I disconnect in one minute or more, there is no error

@alechkos
Copy link
Collaborator Author

alechkos commented Jan 22, 2024 •
edited

@ekapasha17

nobody told me, but based on my experience when I disconnect under one minute I get the error but when I disconnect in one minute or more, there is no error

How exactly did you test the pr?

not solving the problem yet

Until you provide complete information these are just words.
Use this guide to report an issue with a pr

@ekapasha17
Copy link

@ekapasha17

nobody told me, but based on my experience when I disconnect under one minute I get the error but when I disconnect in one minute or more, there is no error

How exactly did you test the pr?

not solving the problem yet

Until you provide complete information these are just words. Use this guide to report an issue with a pr

I scan the phone, and I tried to logout after the phone authenticated and suddenly I get the error but when I scan the phone and waiting for one minute and tried to logout, there is no error at all

@alechkos
Copy link
Collaborator Author

@ekapasha17

I scan the phone, and I tried to logout after the phone authenticated and suddenly I get the error but when I scan the phone and waiting for one minute and tried to logout, there is no error at all

Please provide the code so I could test also

@ekapasha17
Copy link

@ekapasha17

I scan the phone, and I tried to logout after the phone authenticated and suddenly I get the error but when I scan the phone and waiting for one minute and tried to logout, there is no error at all

Please provide the code so I could test also

client.on('disconnected', (reason) => { console.log(WhatsApp disconnected for client: ${clientId} reason: ${reason}`);

    // Use setTimeout to delay the reinitialization process
    setTimeout(async () => {
        try {
            // Destroy the current client instance
            await client.destroy();
            console.log("Client destroyed successfully");

            // Reinitialize the client instance
            await client.initialize();
            console.log("Client re-initialized successfully");

        } catch (err) {
            console.error(`Error during client destruction or re-initialization: ${err}`);
        }
    }, 5000);

});`

I just used that kind of code

joweste

This comment was marked as off-topic.

Sign in to view

@DukazzCruz
Copy link

Work for me install independent "puppeteer": "^22.6.4".
Try it.

@alechkos alechkos marked this pull request as draft April 16, 2024 02:53
Repository owner locked and limited conversation to collaborators Apr 16, 2024
Repository owner unlocked this conversation Apr 19, 2024
@alechkos alechkos marked this pull request as ready for review April 27, 2024 19:25
@alechkos
Copy link
Collaborator Author

@tofers

pupBrowser.on('disconnected'

This event fires when the browser is closed. It is also triggered when

client.destroy();
or
pupBrowser.close()

In this code, you are simply killing the session, which will be deleted on any browser crash.

No, the session won't be deleted on every browser crash, but only when you purposly call await client.logout() as is shown in the How Has the PR Been Tested section.
Along with the deletion of a session folder, the linked session on the phone is terminated successfully as it supposed to be if you want to logout from your web session.

@themazim
Copy link

themazim commented Apr 28, 2024 •
edited by alechkos

No, the session won't be deleted on every browser crash, but only when you purposly call await client.logout() as is shown in the How Has the PR Been Tested section. Along with the deletion of a session folder, the linked session on the phone is terminated successfully as it supposed to be if you want to logout from your web session.

can confirm. Since applying this PR the "hanging" sessions issue was resolved.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@joweste joweste joweste left review comments

@shirser121 shirser121 shirser121 left review comments

@themazim themazim themazim approved these changes

@Davixe7 Davixe7 Davixe7 approved these changes

@Adityasundawa Adityasundawa Adityasundawa approved these changes

At least 2 approving reviews are required to merge this pull request.

Assignees
No one assigned
Labels
breaking change Fix or feature that would cause existing functionality to change
Projects
None yet
Milestone
No milestone
9 participants
@alechkos @jacopobonomi @ekapasha17 @DukazzCruz @themazim @Davixe7 @joweste @Adityasundawa @shirser121