Reason for calling puppet.logout() in puppet.stop()?

[hcfw007]

puppet/src/mixins/login-mixin.ts

Lines 46 to 51 in b5c101c

	override async stop (): Promise<void> {
	log.verbose('PuppetLoginMixin', 'stop()')
	await this.logout()
	await super.stop()
	}

We noticed that in login-mixin, the stop() method calls logout(). Could you explain the reason for this behavior?
As far as I understand, if we stop a puppet, when we restart it, the session should remain and we don't need to login again. So there is no need to call logout() in stop().

[huan]

The reason for calling puppet.logout() in puppet.stop() is that, the Wechaty program need to maintain the state of the Chatbot session status of whether it is logged in or not.

When the Wechaty has been stopped, the puppet system can not garantee the login state will not be changed before next wechaty start, which means that if the Wechaty do not emit the logout event to the program, the program will be highly possible in an incorrect state.

That's the reason we decided to emit the logout event when we calling stop.

In the current design, when the Wechaty start, it will either emit the login event if the bot has the logged in status, or it will emit scan event if the bot account need to be scanned to logged in.

The above design will make sure the Chatbot program can maintain a correct log in status of the bot state.

I hope it can answer your question, please feel free to let me know if you have more questions.

[hcfw007]

OK. So if my understanding is right, the puppet should not override the logout method of puppet? Where should we put the actual puppet logout implementation?

[huan]

So if my understanding is right, the puppet should not override the logout method of puppet?

Your understanding is not correct.

A puppet provider implementation can override the logout method as long as it calls the super.logout() for chain the base implementation.

Where should we put the actual puppet logout implementation?

I hope the above comment also answer this question.

[hcfw007]

So stop() calling logout() is for wechaty state maintenance. For puppet implementation we should add this just like puppet.logout() if we want to keep IM session? (this works, but i'm not sure if it's best practice)

if (!this.isLoggedIn) {
      log.verbose(PRE, 'logout() do nothing')
      return
    }

[huan]

Where do you want to our this code block into?

Please provide more information to help me understand your design.

[hcfw007]

OK. As you know we are trying to merge v0.41 puppet-whatsapp to main. When upgrading wechaty version to 1.x, we found that puppet.stop() calls puppet.logout(). It caused two problems.

1. Whatsapp client has already been destroyed in stop process and logout will throw an error when trying to call methods on whatsapp client.
2. We don't really want to logout. Calling logout() will clear the session and next time we have to scan QR to login. This is not what we want to do in stop()

[huan]

Thanks for the detailed description.

I think your two points are very valuable and should be considered.

I'll think about it and update back to you soon.

[huan]

After some time of thinking about this situation, I think we should stop calling logout() inside the stop() by just emit a logout event from the puppet.

Changing by this, the problems 1 & 2 that you have described will be solved, and the logout & login/scan state changes during the puppet start/stop can be managed correctly.

Please feel free to let me know what you think, and we can create a PR to solve those problems after we have the agreement.