broidHQ / integrations

Connect your App to Multiple Messaging Channels with the W3C Open standard.
https://www.broid.ai
GNU Affero General Public License v3.0
746 stars 83 forks source link

messenger: data.actor should have required property 'id' #78

Closed acailly closed 7 years ago

acailly commented 7 years ago

Overview

broid-messenger - bug

Details

User id is not set in activitystreams.actor field, causing schema validation to fail.

Steps to Reproduce

Start a bot using broid-messenger Talk to it

Observed

I have this error:

{"pid":10836,"hostname":"user-PCXXX","name":"parser","level":50,"time":1489086028378,"msg":"data.actor should have required property 'id'","type":"Error","stack":"Error: data.actor should have required property 'id'\n    at Promise (C:
..
}

After investigation, I found that activitystreams.actor.id is set in parser.js-parse(event):

parse(event) {
        ...
        const activitystreams = this.createActivityStream(normalized);
        activitystreams.actor = {
            id: R.path(["authorInformation", "id"], normalized), <--- HERE
            name: broid_utils_1.concat([R.path(["authorInformation", "first_name"], normalized),
                R.path(["authorInformation", "last_name"], normalized)]),
            type: "Person",
        };
       ...

... and that the value comes from authorInformation, which is set in adapter.js-listen():

listen () {
    return Rx_1.Observable.fromEvent(this.webhookServer, 'message')
            .mergeMap((event) => this.parser.normalize(event))
            .mergeMap((messages) => Rx_1.Observable.from(messages))
            .mergeMap((message) => this.user(message.author) <-- COMPUTED HERE
            .then((author) => {
              message.authorInformation = author <--- SET HERE
              return message
            }))
            ...

... from the return value of adapter.js-user(...):

user (id, fields = 'first_name,last_name', cache = true) {
    const key = `${id}${fields}`
    if (cache && this.storeUsers.get(key)) {
        const data = this.storeUsers.get(key)
        return Promise.resolve(data)
      }
    return rp({
        json: true,
        method: 'GET',
        qs: { access_token: this.token, fields },
        uri: `https://graph.facebook.com/v2.8/${id}`
      })
            .then((data) => {
              this.storeUsers.set(key, data)
              return data <-- HERE
            })
  }

But as we can see in messenger doc, id is not returned in this request: https://developers.facebook.com/docs/messenger-platform/user-profile

Expected

The field id is set, the following modification does the trick but I didn't test it a lot:

user (id, fields = 'first_name,last_name', cache = true) {
    const key = `${id}${fields}`
    if (cache && this.storeUsers.get(key)) {
        const data = this.storeUsers.get(key)
        return Promise.resolve(data)
      }
    return rp({
        json: true,
        method: 'GET',
        qs: { access_token: this.token, fields },
        uri: `https://graph.facebook.com/v2.8/${id}`
      })
            .then((data) => {
              data.id = id <--HERE
              this.storeUsers.set(key, data)
              return data
            })
  }
killix commented 7 years ago

thanks, I will take a look on that today.

killix commented 7 years ago

The fix will be merge after the review. We will publish a new version of the package right after. Thanks again @acailly !

killix commented 7 years ago

Weird think happened with Github (about #79). This is why we have now #80

killix commented 7 years ago

fixed with version 1.1.7