microsoft / BotFramework-Services

Microsoft Bot Framework Services
Creative Commons Attribution 4.0 International
38 stars 11 forks source link

GroupMe channel returns HTTP status 500 when sending Image or Location via ChannelData #101

Closed sphanley closed 5 years ago

sphanley commented 5 years ago

I'm building a Bot Framework bot that's connected to the GroupMe channel. According to the (non Bot Framework specific) GroupMe Bot tutorial, GroupMe supports bots sending Image or Location attachments, by sending a message with the following syntax:

{
  "bot_id"  : "j5abcdefg",
  "text"    : "Hello world",
  "attachments" : [
    {
      "type"  : "image",
      "url"   : "https://i.groupme.com/somethingsomething.large"
    }
  ]
}

Or:

{
  "bot_id"  : "j5abcdefg",
  "text"    : "Hello world",
  "attachments" : [
    {
      "type"  : "location",
      "lng"   : "40.000",
      "lat"   : "70.000",
      "name"  : "GroupMe HQ"
    }
  ]
}

When my bot receives a message from a human user that contains a location attachment, it comes through like this:

{
   ... 
   "channelData":[  
      {  
         "type":"location",
         "lat":"70.000",
         "lng":"40.000",
         "name":"GroupMe HQ"
      }
   ],
   ...
}

Based on that, I'm trying to send a location from my bot using the following code:

var msg = MessageFactory.Text(messageText);
msg.ChannelData = new dynamic[]
{
    new {
        type = "location",
        lng = "40.000",
        lat = "70.000",
        name = "GroupMe HQ"
    }
};

await turnContext.SendActivityAsync(msg);

However, this results in a 500 status code from my bot's endpoint, printing the following in chat:

Service Error:POST to {BotName} failed: POST to the bot's endpoint failed with HTTP status 500

Or, occasionally:

Service Error:POST to {BotName} timed out after 15s

If there's something wrong with my syntax, I'd appreciate a correction - otherwise, I suspect that there's a defect in the way that the GroupMe channel service is processing ChannelData.

sphanley commented 5 years ago

Per this issue on the BotBuilder SDK repo, I've also tried the following code:

msg.ChannelData = JObject.FromObject(new[]
{
    new {
        "type"  : "location",
        "lng"   : "40.000",
        "lat"   : "70.000",
        "name"  : "GroupMe HQ"
    }
});

and

msg.ChannelData = JObject.FromObject(new
{
    attachments = new[]
    {
        new
        {
            "type"  : "location",
            "lng"   : "40.000",
            "lat"   : "70.000",
            "name"  : "GroupMe HQ"
        }
    }
});

The first snippet doesn't work at all, even in the emulator - it causes an exception "ArgumentException: Object serialized to Array. JObject instance expected."

The second snippet works in that it doesn't cause a 500 error, but no attachment is received in GroupMe.

sphanley commented 5 years ago

I've now tried what I think is the most explicitly correct syntax. Given the following model classes:

[Serializable]
public class GroupMeChannelData : JArray
{
    public void Add(GroupMeAttachment attachment)
    {
        Add(JToken.FromObject(attachment));
    }
}

[Serializable]
public class GroupMeAttachment
{
    [JsonProperty("type")]
    virtual public string Type { get; set; }
}

[Serializable]
public class GroupMeLocationAttachment : GroupMeAttachment
{
    [JsonProperty("type")]
    public override string Type => "location";
    [JsonProperty("lat")]
    public string Latitude { get; set; }
    [JsonProperty("lng")]
    public string Longitude { get; set; }
    [JsonProperty("name")]
    public string Name { get; set; }
}

I generated the attachment using as below:

var msg = MessageFactory.Text(messageText);
var channelData = new GroupMeChannelData
{
    new GroupMeLocationAttachment()
    {
        Latitude = "70.000",
        Longitude = "40.000",
        Name = "GroupMe HQ"
    }
};
msg.ChannelData = channelData;

await turnContext.SendActivityAsync(msg);

In the Bot Framework Emulator, this sends a message with ChannelData formatted exactly as I'm led to believe GroupMe is expecting, based on the GroupMe docs as well as the ChannelData I see when my bot receives attachments from non-bot users in the chat. However, this still causes a 500 error when sent to the actual GroupMe channel. Based on that, I feel strongly that this is a defect in the connector service.

anusharr commented 5 years ago

Hello @sphanley, if your goal is to send a location from the bot, you can instantiate an HTTP client and then make the api calls. Also, can you try using the code below to see if you are able to send location from the bot.

var reply = MessageFactory.Attachment(new Attachment("image/png", "https://i.imgur.com/vta1imR.png"));
            reply.ChannelData = JObject.FromObject(new { attachment = new {
                    type = "location",
                    lng = "40.000",
                    lat = "70.000",
                    name = "GroupMe HQ"
                }
            });

           await turnContext.SendActivityAsync(reply);

I tried using the above code and it does send the location.

sphanley commented 5 years ago

Thanks for the response, @anusharavindrar! You're right, if I use the snippet you posted, I do see both the image and location attachments come through in my GroupMe chat. However, if I try to remove the Image attachment and only add the location attachment to a message that otherwise only contains text, I receive a 500 error. Can you try that and let me know if you see the same? I would expect that to be valid. Here's the code:

        var reply = MessageFactory.Text("Location-only test");
        reply.ChannelData = JObject.FromObject(new { attachment = new {
                type = "location",
                lng = "40.000",
                lat = "70.000",
                name = "GroupMe HQ"
            }
        });

       await turnContext.SendActivityAsync(reply);

I'd really love to avoid having to spin up an HTTPClient and make API calls directly - that's the whole reason I'm using the Bot Framework to begin with, you know?

It's also strange to me that the syntax that seems to work for ChannelData is adding a single object to the attachment key, since when the bot receives a message with an attachment in the ChannelData, it comes through as an attachments (plural) array containing the single element, which is in keeping with what's shown in the GroupMe API docs.

sphanley commented 5 years ago

Additionally, when I do use your sample code and send the message containing the image attachment, somehow the JSON received is this:

attachments: [
    {type: "image", url: "https://i.groupme.com/800x1412.png.231291571d6c4432bb61a7e2f9578421"},
    {lat: "70.000", lng: "40.000", name: "GroupMe HQ", type: "location"}
]

So somehow, the channel connector is inferring that this is an image, replacing the image/png type with image, and then reuploading it to GroupMe's image host? It feels like there's just a lot of undocumented behavior happening inside the connector channel service.

anusharr commented 5 years ago

Correct. If you use just the location attachment to the message, it throws up an error. I did try the code which you pasted above and encountered the same issue before coming up with the workaround which I shared. There seems to be an issue with the GroupMe connector and how it is processing the channelData.

sphanley commented 5 years ago

Ok! Just wanted to confirm that we were on the same page. Strange that adding the image Attachment allows the channelData location attachment to be correctly processed as well.

Interestingly, it seems that if I try to additionally assign anything to the message.Text property in the snippet you posted, it also causes the 500 error to be thrown, even with both the image and location attachments present. Per the GroupMe bot tutorial, it should be syntactically valid to send a message which contains both text and attachments, so that looks like a bug as well.

daveta commented 5 years ago

Hi @sphanley, thanks for reporting the original issue. Could you open up a separate issue tracking the message.text issue if you are still experiencing the problem?

sphanley commented 5 years ago

Hi @daveta! I'm happy to help split this into multiple issues, but I'm honestly not 100% sure how you'd like me to break them up. The way I look at it, the overarching issue is that the Bot Framework GroupMe service doesn't properly support attachments. Within that, there's sort of several sub-issues:

Can you give me any guidance as to which of these pieces you'd like split into follow-up issues? Ideally, since several of them are sort of interconnected, I'd think they'd all need to be fixed in one major pass at ensuring GroupMe Attachments work correctly overall - but I'm happy to open additional issue(s), however works best for you.

daveta commented 5 years ago

Hi @sphanley, I've contacted the developer that owns the code, we're going to dig in. Thanks again for the time reporting. We'll get back to you.

sphanley commented 5 years ago

Sounds great. If you need any clarification or reproduction steps for any of this, please let me know - I really appreciate the level of support you all are providing given that I'm doing a small project on a low-traffic channel!

hcyang commented 5 years ago

hi @craigjensen and @jameslew, any updates?

sphanley commented 5 years ago

Seconding the request for any updates - I'm working on a GroupMe Bot Framework project and have features I'd love to move forward with, but the largely broken attachment support is really limiting. Thanks for your work on resolving this issue!

carlosscastro commented 5 years ago

@sphanley this is being investigated, we'll provide an update once we have a concrete plan. We apologize for the delay, and completely agree that broken attachment support is a limiting factor. Thanks for your patience!

sphanley commented 5 years ago

Thanks for the update @carloscastro! I appreciate the ongoing investigation.

arturl commented 5 years ago

Thank you for the issue report and working with us to get to the detail. We've evaluated the issue and determined that it doesn't fit in our current semester's schedule. We have added it to our backlog for future planning, but do not have an ETA to offer at this time.

sphanley commented 5 years ago

Gotta say, this is really disappointing, especially considering that GroupMe is a first-party Microsoft channel - I understand that it's likely a low-traffic channel, and fully addressing this such that GroupMe attachments are well-supported by the framework is a big lift, but I'd hope that you'd at least consider prioritizing fixing the broken channel connector such that it's possible to send dynamic attachments via ChannelData, like is done for unsupported attachments on other channels. If this and #97 are both just going to be closed without any remediation, then the GroupMe channel is fundamentally unusable for a huge swath of very basic and important channel functionality.

It's a shame that the channel connectors (or at least those that don't depend on any undocumented APIs) aren't open source like the core framework - were that the case, I'd personally take a stab at getting at least minimal viable attachment support functioning correctly.