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

fix(channel-messenger): fix initialization #3011

Merged
merged 3 commits into from Feb 27, 2020
Merged

fix(channel-messenger): fix initialization #3011

merged 3 commits into from Feb 27, 2020

Conversation

@asashour
Copy link
Contributor

asashour commented Feb 27, 2020

As hinted in the Forum, the channel-messenger doesn't currently work, it simply hangs during initialization.

The reason is because router.getPublicPath() awaits AppLifecycle.waitFor(AppLifecycleEvents.HTTP_SERVER_READY).

The fix is to call initialize() onServerReady instead of onServerStarted

@asashour asashour force-pushed the asashour:fb branch from 3df085e to 3c2f669 Feb 27, 2020
Copy link
Member

allardy left a comment

Yeah, considering the lifecycle, everything linked to API or routes must wait for the onServerReady, because express is not setup yet when the start occurs. Thank you !

image

@allardy

This comment has been minimized.

Copy link
Member

allardy commented Feb 27, 2020

I'm just wondering if any of the methods in the onBotMount may cause Messenger to try to contact Botpress via the webhook, because this could be another issue.

The cycle is onServerStarted -> Mount Bots -> OnServerReady

If FB calls the webhook in the mount phase, this may not work. I think the best fix would be to leave getPublicPath as a floating promise, eg

image

What do you think ?

I'm wondering why we didn't hear about this earlier, the getPublicPath was changed a year ago, and messenger was not changed since

@asashour

This comment has been minimized.

Copy link
Contributor Author

asashour commented Feb 27, 2020

I'm wondering why we didn't hear about this earlier, the getPublicPath was changed a year ago, and messenger was not changed since

I thought some recent change of lifecycle happened, otherwise it is really strange.

The case I tested, was without Facebook knowing about Botpress at all, I created a Facebook page, then used its configuration in Botpress .

The cycle is onServerStarted -> Mount Bots -> OnServerReady

When I tested, I see something different, it seems that it is onServerStarted, then onServerReady, then onBotMount, so there is no need to use onServerStarted with floating promise, since onServerReady can be used before bot mounting.

@allardy

This comment has been minimized.

Copy link
Member

allardy commented Feb 27, 2020

Oh yeah right, I forgot about the floating promise. On ready is called once the server accepts connection, then it calls the on ready

@asashour asashour force-pushed the asashour:fb branch from 1b875f8 to 7b85f42 Feb 27, 2020
Copy link
Member

allardy left a comment

Excellent, thanks !

@allardy allardy merged commit 348227a into botpress:master Feb 27, 2020
5 checks passed
5 checks passed
Run Prettier on codebase
Details
Unit
Details
Run TSLint on codebase
Details
e2e
Details
license/cla Contributor License Agreement is signed.
Details
@asashour asashour deleted the asashour:fb branch Feb 28, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

None yet

2 participants
You can’t perform that action at this time.