benjifs / bitbar-slack-team-notifications

Show notifications for Slack teams and channels with option to mark as read
MIT License
11 stars 0 forks source link

Occasional timeout errors #4

Open futzlarson opened 3 years ago

futzlarson commented 3 years ago

I see this sometimes:

image

I think it usually goes away on the next check. Might be good to ignore it if it's a Slack API issue until it crosses some threshold? This is a tool I've been wanting for years so I hope to help improve it as I notice things.

benjifs commented 3 years ago

I think this one might be tricky. As far as I know, Bitbar requests a full refresh every time the script runs. By default in this case that means that every minute it has to wipe and print the information again.

I could probably also save the most recent valid output and wait until a few invalid requests to present it but that would require adding another dependency to do that.

Not sure what the best move is here but I think it might be best to request this be added to Bitbar and have a way for the plugin to fail without losing the previous output.

futzlarson commented 3 years ago

Would you mind putting that request in? I think you could describe it better, as the developer.

benjifs commented 3 years ago

I'm waiting for a reply in the Bitbar Slack. Will wait to see if there's any response and will post it as an issue later if I don't hear back

futzlarson commented 3 years ago

I also wonder if you could just ignore these errors. They seemingly only happen when there is an issue w/ the Slack API's response, which you can't do anything about anyway? They seem to only last one cycle anyway, but the ! gets my attention.

benjifs commented 3 years ago

I could probably handle a timeout error separately and instead of showing the error have a "Retrying in Xs..."

Have you tried changing the frequency the plugin is updating to prevent the timeout from happening? Maybe switching from x.1m.js to x.90s.js or even higher?

futzlarson commented 3 years ago

Ha I was wondering what the 1m was about, and was wondering how to change the frequency. Is that a BitBar standard? If not, seems like all options could be in one config file, where you specify tokens and any other options like MAX_LENGTH. Not a big deal though.

One note about the frequency and renaming the file - that's inconvenient for updates because I would need to rename the file back slack-team-notifications.1m.js then do a git pull, then rename back to slack-team-notifications.5m.js. Btw, I already ran into a timeout error even at the 5-minute setting.

benjifs commented 3 years ago

Yeah, the 1m thing is from Bitbar and its the general way or plugins define frequency.

That's weird that it would happen even at a greater interval. It might be a rate limiting thing from Slack or something else. The way that this plugin works is that it make one request per channel to get the unread count so if you have 100+ channels, there might be some limit issues which are defined by Slack. Not sure if thats the case, just a theory.

Btw, the reply I got about keeping an old output in case of error is that its the plugins responsibility which in this case would involve adding another dependency like node-persist to save states. Not sure if that's worth it for just this but this might also help get rid of some duplicate calls that are made every run even to get channel_list and other constant data.

futzlarson commented 3 years ago

Bummer about the frequency. It's an odd way to handle it on their part, given the git pull issue I mentioned.

It's not strange to me that the errors happens at a greater interval. I think it's just Slack API issues, right? And for the record, I have a small number of channels. 4 in one and 3 in the other. So to me the simple answer is to just ignore timeout errors. Maybe treat them as success and display nothing, since there's nothing you can do about that anyway. I would prefer that over a ! that catches my eye and goes away on the next update (which is now 5 full minutes). It's not a useful error to see.

Up to you on keeping old output, but it does sound like that could also reduce the timeout errors.

futzlarson commented 3 years ago

Also seeing this from time to time:

image

benjifs commented 3 years ago

Not sure why this is happening as I only get the ENOTFOUND error when my network goes down or similar situations. I still haven't seen the timeout error so not sure how to handle it on my end without knowing what the response looks like.

I just added some new changes but messed up my git history while rebasing so if you have any issues pulling the latest changes, just do a fresh checkout. Sorry about that.

The changes clean up the API calls, config variables at the top, added a way to add a workspace at any time, and show the workspace even if there are no new notifications.

futzlarson commented 3 years ago

I did get the git error, but also an error on a new checkout (with .tokens.js copied over):

$ ./slack-team-notifications.5m.js 
node:internal/modules/cjs/loader:928
  throw err;
  ^

Error: Cannot find module 'superagent'
Require stack:
- /Users/rduval/Code/bitbar/bitbar-slack-team-notifications/slack-team-notifications.5m.js
    at Function.Module._resolveFilename (node:internal/modules/cjs/loader:925:15)
    at Function.Module._load (node:internal/modules/cjs/loader:769:27)
    at Module.require (node:internal/modules/cjs/loader:997:19)
    at require (node:internal/modules/cjs/helpers:92:18)
    at Object.<anonymous> (/Users/rduval/Code/bitbar/bitbar-slack-team-notifications/slack-team-notifications.5m.js:13:17)
    at Module._compile (node:internal/modules/cjs/loader:1108:14)
    at Object.Module._extensions..js (node:internal/modules/cjs/loader:1137:10)
    at Module.load (node:internal/modules/cjs/loader:973:32)
    at Function.Module._load (node:internal/modules/cjs/loader:813:14)
    at Function.executeUserEntryPoint [as runMain] (node:internal/modules/run_main:76:12) {
  code: 'MODULE_NOT_FOUND',
  requireStack: [
    '/Users/rduval/Code/bitbar/bitbar-slack-team-notifications/slack-team-notifications.5m.js'
  ]
}

Let me know what line to add and where for more info for the timeout.

benjifs commented 3 years ago

Running npm install or npm ci should handle installing the dependencies needed

futzlarson commented 3 years ago

Ha, duh.

Let me know what line to add and where for more info on the timeout. We can write it to file or something and I can grab it next time I see it.