wechaty / puppet-padlocal

Puppet PadLocal is a Pad Protocol for WeChat
https://wechaty.js.org/docs/puppet-providers/padlocal
Apache License 2.0
642 stars 88 forks source link

Cannot read properties of undefined (reading 'getRoomMember') #115

Closed huan closed 2 years ago

huan commented 2 years ago

When we using PadLocal puppet, call stop() then call start() again, we will get this error:

12:54:00 ERR Config process.on(unhandledRejection) promise.catch(13 INTERNAL: Cannot read properties of undefined (reading 'getRoomMember'))

It's because we set this._cacheMgr to undefined in stop():

https://github.com/wechaty/puppet-padlocal/blob/a4b04b723fc4f75d08596a2952e08f4b5037983c/src/puppet-padlocal.ts#L297-L298

but there's no initialization code in start():

https://github.com/wechaty/puppet-padlocal/blob/a4b04b723fc4f75d08596a2952e08f4b5037983c/src/puppet-padlocal.ts#L140-L142

So when we call start() again, the _cacheMgr will be undefined and we will run into this issue:

https://github.com/wechaty/puppet-padlocal/blob/a4b04b723fc4f75d08596a2952e08f4b5037983c/src/puppet-padlocal.ts#L1361-L1362

Related issue

padlocal commented 2 years ago

Current implementation may be right logically. The _cacheMgr will be re-inited at https://github.com/wechaty/puppet-padlocal/blob/a4b04b723fc4f75d08596a2952e08f4b5037983c/src/puppet-padlocal.ts#L259-L272 So the process: start -> begin login -> login done(setup _cacheMgr) -> ... -> stop(teardown _cacheMgr) -> ... -> start

The real problem is: getRoomMember has been called after stop, but before login done. And any api has to be called while user is signed on, the behavior is undefined without an valide user session.

Even if we fix this issue by moving this._cacheMgr = new CacheManager(userId); into _startClient, the logic is still broken.

@huan I think it's better to manage api call against bot state at Wechaty engine level.

huan commented 2 years ago

I have read your message and have been wondering whether we can use a quick workaround for this problem?

For example, can we just return a empty array when the cache manager is not initialized:

-    let ret = await this._cacheMgr!.getRoomMember(roomId); 
+    let ret = await this._cacheMgr?.getRoomMember(roomId) || []; 

I think an empty return is acceptable before the bot logged in.

BTW: Using a ! without a guard logic is an anti-pattern, I think we should not use ! as possible as we can because it will leave us without any static typing protection.

padlocal commented 2 years ago

Nice suggestion! And workaround has been applied.