ServiceNowDevProgram / Points-Thing

The official application repository for the bot @Points-Thing on the sndevs.com workspace.
https://github.com/ServiceNowDevProgram/Hacktoberfest
4 stars 57 forks source link

Resolve chat parser error that occurs occasionally #18

Closed earlduque closed 1 year ago

earlduque commented 1 year ago
com.glide.script.RhinoEcmaError: The undefined value has no properties.
sys_script.692ca881dba29150791d8f8d139619ee.script : Line(5) column(0)
2:
3: var message = JSON.parse(current.payload);
4:
==> 5: var userSlackID = message.event.user.replace(/<@/g,"").replace(/>/g,"");
6: var userSysID = new x_snc_pointsthing.PointsThing().establish_user(userSlackID, false);
7:
8: if (!message.event.text || !userSysID) return;

unsure of what is causing it

SapphicFire commented 1 year ago

Hey Earl,

I haven't been able to generate a message that doesn't have the .user or (unlikely) the .event object, would you be able to search for sys_created_onONToday@javascript:gs.beginningOfToday()@javascript:gs.endOfToday()^payloadNOT LIKE"user": on the payload table and give us some examples I can work with, if that's OK? We can test for the missing attribute easily enough, but being able to assess the model would make sure we can have a more permanent solution validated.

SapphicFire commented 1 year ago

Ok, found it. Slack has a couple of message payload formats that won't have the user property directly within the event object. The most likely trigger is message_changed (people updating their messages).

Reason for error 🔍:

The payloads that don't work have the subtype property under event, and only the following subtypes change format:

Proposed fix 🛠️:

Test for the user or subtype property and if present, test the subtype and change where you draw slackID from respectively

Code snippet:

var userSlackID, message = JSON.parse(current.payload);
if (current.event.hasOwnProperty('subtype') && (current.event.subtype === "message_changed" /* || current.event.subtype === "message_replied" */)) {
    userSlackID = message.event.message.user.replace(/<@/g,"").replace(/>/g,"");
}
userSlackID = message.event.user.replace(/<@/g,"").replace(/>/g,"");

Conversation 🗣️:

Implementing that would mean we would support points on edit, which could add a chance to be gamed (although we could report on that, or set limits).

Alternatively we could add checks for it, and then return without inserting the chats so we avoid the error and triggering actions off the chats. Would be interested for input on preferences for how we implement that?

earlduque commented 1 year ago

Thanks @SapphicFire for the findings, thank you for figuring it out. I added a an ignore on the scripted rest api, the payload is no longer created on message_changed events

if (request.body.data.event.bot_id || request.body.data.event.subtype == 'message_changed') return;
chelming commented 1 year ago

:ohno: this will break trebek not firing multiple times because that was relying on the message_changed event from the image unfurling.

chelming commented 1 year ago

but... that was a hack anyway 😄

I think we should store/process request.body.data.event.bot_id so we can see if/when the bot last posted a message but that'd require updates to basically ignore them in all other instances.

perhaps adding a u_is_bot on the user record and not firing if the message was from a bot?

SapphicFire commented 1 year ago

This shouldn't have any bearing on trebek since we're talking PointsThing not Slacker 😜

earlduque commented 1 year ago

you got TOLD @chelming 🔥

chelming commented 1 year ago

nothing_to_see_here.gif