Attempting to use the SDK when Slack is down will crash the Node process #586

wilg · 2018-06-27T18:57:00Z

Description

Perhaps I'm just missing the proper way to handle these errors, but as far as I can tell if Slack is offline or unreachable in various ways (as it was this morning), the Slack SDK will terminate the Node process on use and you cannot catch the errors.

This looks to be due to the use of a setInterval within the got dependency.

TypeError: "string" must be a string, Buffer, or ArrayBuffer
at Function.byteLength (buffer.js:441:11)
at Timeout.setInterval [as _onTimeout] (/code/node_modules/got/index.js:207:35)
at ontimeout (timers.js:488:11)
at tryOnTimeout (timers.js:323:5)
at Timer.listOnTimeout (timers.js:283:5)

Also, simply disabling the internet connection causes this crash:

Error: getaddrinfo ENOTFOUND slack.com slack.com:443
    at errnoException (dns.js:50:10)
    at GetAddrInfoReqWrap.onlookup [as oncomplete] (dns.js:91:26)

What type of issue is this? (place an `x` in one of the `[ ]`)

Requirements (place an `x` in each of the `[ ]`)

I've read and understood the Contributing guidelines and have done my best effort to follow them.
I've read and agree to the Code of Conduct.
I've searched for any related issues and avoided creating a duplicate issue.

Bug Report

Filling out the following details about bugs will help us solve your issue sooner.

Reproducible in:

@slack/client version: 4.2.2

node version: v8.1.4

OS version(s): macOS 10.13.3 and linux

Steps to reproduce:

Turn off your internet connection
Wrap the Slack SDK calls in try/catch and handle promise rejections.
Attempt to run the code.

Expected result:

A catchable error would be thrown or a callback would fail.

Actual result:

Process crashes.

Attachments:

none

The text was updated successfully, but these errors were encountered:

aoberoi · 2018-06-28T00:28:53Z

Were you using the RTMClient or the WebClient?

wilg · 2018-06-28T01:07:58Z

WebClient

aoberoi · 2018-06-28T02:06:45Z

seems like this is the fix we'd like to see land in got: sindresorhus/got#490

lukechilds · 2018-07-03T02:23:09Z

That fix is merged into Got. Will publish a new release shortly.

aoberoi · 2018-07-03T06:08:48Z

thanks so much @lukechilds!

lukechilds · 2018-07-03T06:28:47Z

I don't actually have permissions to publish to npm but I've looped in the rest of the team.

lukechilds · 2018-07-03T09:15:57Z

@aoberoi Done.

aoberoi · 2018-07-03T12:15:06Z

@wilg i think this issue is now fixed. you should be able to get the latest updates by running npm update in your project. can you confirm that it works for me? even though slack isn't down, your second example (not being connected to the network) should be sufficient.

wilg · 2018-07-03T17:43:05Z

Thanks @aoberoi! I set"@slack/client": "^4.3.1", but still get the same error message when I disable the internet to my app. I have all the Slack calls wrapped in try/catch.

This is the entire stacktrace:

Error: getaddrinfo ENOTFOUND slack.com slack.com:443
    at errnoException (dns.js:50:10)
    at GetAddrInfoReqWrap.onlookup [as oncomplete] (dns.js:91:26)

aoberoi · 2018-07-23T20:02:42Z

@wilg sorry for my delay in response! somehow this one got buried for me.

can you do an npm ls got to see which version of got is in your dependency tree? You want it to be at least 8.3.2.

wilg · 2018-07-24T17:46:24Z

This package only asks for "got": "^8.0.3", https://github.com/slackapi/node-slack-sdk/blob/master/package.json#L62 Shouldn't that be "^8.3.2"?

My project (using Yarn) says:

› yarn list got
yarn list v1.3.2
warning Filtering by arguments is deprecated. Please use the pattern option instead.
├─ got@8.3.1
└─ package-json@4.0.1
   └─ got@6.7.1
✨  Done in 0.49s.

aoberoi · 2018-07-24T18:06:52Z

@wilg we could make that change, but according to semver the caret range will match all releases less than 9.0.0. So we shouldn’t need to make any changes to pick up that release up.

It’s likely that you have a lock file that’s preventing your yarn from matching to the latest got release. You may need to manually run yarn upgrade to re-resolve to the latest.

What’s confusing to me is that your shown tree doesn’t include got inside of @slack/client. Can you explain why?

wilg · 2018-07-24T18:12:51Z

Oh yeah, duh. I assumed for some reason that doing yarn install after bumping the slack-client version would have updated the dependencies also, but I think it makes sense that it wouldn't.

I'm not sure why it doesn't show up in yarn list. In yarn.lock it shows as a dependency:

"@slack/client@^4.3.1":
  version "4.3.1"
  resolved "https://registry.yarnpkg.com/@slack/client/-/client-4.3.1.tgz#5caa1eaf753e4fcd23d8c04bcdc215336867a233"
  dependencies:
    "@types/delay" "^2.0.1"
    "@types/form-data" "^2.2.1"
    "@types/got" "^7.1.7"
    "@types/is-stream" "^1.1.0"
    "@types/loglevel" "^1.5.3"
    "@types/node" "^9.4.7"
    "@types/p-cancelable" "^0.3.0"
    "@types/p-queue" "^2.3.1"
    "@types/p-retry" "^1.0.1"
    "@types/retry" "^0.10.2"
    "@types/url-join" "^0.8.2"
    "@types/ws" "^5.1.1"
    delay "^2.0.0"
    eventemitter3 "^3.0.0"
    finity "^0.5.4"
    form-data "^2.3.1"
    got "^8.0.3"
    is-stream "^1.1.0"
    loglevel "^1.6.1"
    object.entries "^1.0.4"
    object.getownpropertydescriptors "^2.0.3"
    object.values "^1.0.4"
    p-cancelable "^0.3.0"
    p-queue "^2.3.0"
    p-retry "^1.0.0"
    retry "^0.10.1"
    url-join "^4.0.0"
    ws "^5.2.0"

Maybe its just how yarn list works? The full project is here: https://github.com/looker/actions/blob/master/yarn.lock

I was able to properly bump the version with yarn upgrade @slack/client.

Making sure the version bump fixes the issue now...

wilg · 2018-07-24T18:25:13Z

Unfortunately my app still crashes the same way with got 8.3.2:

Error: getaddrinfo ENOTFOUND slack.com slack.com:443
    at errnoException (dns.js:50:10)
    at GetAddrInfoReqWrap.onlookup [as oncomplete] (dns.js:91:26)
[nodemon] app crashed - waiting for file changes before starting...
^C

ruby-2.3.3 in actions/ on master 
› yarn list got     
yarn list v1.3.2
warning Filtering by arguments is deprecated. Please use the pattern option instead.
├─ got@8.3.2
└─ package-json@4.0.1
   └─ got@6.7.1
✨  Done in 0.49s.

The code that's crashing is here (calling the execute method) https://github.com/looker/actions/blob/master/src/actions/slack/slack.ts#L26:L63

aoberoi · 2018-07-24T20:09:15Z

Okay, it looks like we need to reopen the investigation into this issue. Thanks for the helpful info and the link to the code where it occurs! I'm sure that will help us resolve this faster.

Ismaw34 · 2018-07-30T11:25:29Z

No errors found during test on a RocketChat 0.67.0.

aoberoi · 2018-07-31T04:48:22Z

i tried to reproduce this one again. i wasn't able to observe the error. here is what i ran:

const { WebClient, retryPolicies } = require('@slack/client');

const web = new WebClient(
  'xoxp-xxxxxxxxxxxxxxx',
  {
      logLevel: 'debug',
      retryConfig: retryPolicies.fiveRetriesInFiveMinutes,
  }
);

web
  .channels
  .list({
      exclude_members: true,
      exclude_archived: true,
      limit: 10
  })
  .then(res => {
     console.log('channels', res.channels);
  })
  .catch(error => {
      console.error('Error: ', error);
  });

i ran this with the network interface of my machine turned off, and got the following output.

ankur:node-sdk-playground/ $ node web-channel-list.js
[DEBUG] @slack/client:WebClient initialized
[DEBUG] @slack/client:WebClient apiCall() start
[DEBUG] @slack/client:WebClient request attempt
[DEBUG] @slack/client:WebClient request attempt
[DEBUG] @slack/client:WebClient request attempt
[DEBUG] @slack/client:WebClient request attempt
[DEBUG] @slack/client:WebClient request attempt
[DEBUG] @slack/client:WebClient request attempt
Error:  { Error: A request error occurred: ENOTFOUND
    at requestErrorWithOriginal (/Users/ankur/Developer/play/node-sdk-playground/node_modules/@slack/client/dist/WebClient.js:519:5)
    at got.post.then.catch (/Users/ankur/Developer/play/node-sdk-playground/node_modules/@slack/client/dist/WebClient.js:373:31)
    at <anonymous>
    at process._tickCallback (internal/process/next_tick.js:188:7)
  code: 'slackclient_request_error',
  original: 
   { RequestError: getaddrinfo ENOTFOUND slack.com slack.com:443
    at ClientRequest.req.once.err (/Users/ankur/Developer/play/node-sdk-playground/node_modules/got/index.js:182:22)
    at Object.onceWrapper (events.js:315:30)
    at emitOne (events.js:116:13)
    at ClientRequest.emit (events.js:211:7)
    at TLSSocket.socketErrorListener (_http_client.js:387:9)
    at emitOne (events.js:116:13)
    at TLSSocket.emit (events.js:211:7)
    at emitErrorNT (internal/streams/destroy.js:64:8)
    at _combinedTickCallback (internal/process/next_tick.js:138:11)
    at process._tickCallback (internal/process/next_tick.js:180:9)
     name: 'RequestError',
     code: 'ENOTFOUND',
     host: 'slack.com',
     hostname: 'slack.com',
     method: 'POST',
     path: '/api/channels.list',
     protocol: 'https:',
     url: 'https://slack.com/api/channels.list' } }

this is exactly what i would have expected. the printing of the error is from my own catch clause, not from the process crashing.

here's my dependency tree for got:

ankur:node-sdk-playground/ $ npm ls got
sdk-playground@1.0.0 /Users/ankur/Developer/play/node-sdk-playground
└─┬ @slack/client@4.3.1
  └── got@8.3.2

aoberoi · 2018-07-31T04:59:00Z

@wilg not that i think this is the cause of your issue, but i noticed that the following code you linked seemed like it could be cleaned up a little. here's an example:

https://github.com/looker/actions/blob/c9b60baffac86a7228b8c13b0b60f2360e5273a9/src/actions/slack/slack.ts#L46-L61

	let response
	const slack = this.slackClientFromRequest(request)
    try {
		await slack.files.upload(options) // when you don't pass a callback, this returns a Promise
	} catch (e) {
		response = { success: false, message: e.message }
	}
	return new Hub.ActionResponse(response)

wilg · 2018-07-31T18:14:26Z

Interesting, thanks for investigating. I can't reproduce it with your script either (using the same node_modules as my main project), but I am still seeing it crash in my project. I'm trying to figure out what's different. It's definitely crashing during slack.files.upload, but I'm scratching my head for what could be causing it.

aoberoi · 2018-09-13T17:47:09Z

@wilg FYI, the latest versions of this package no longer use got. it might be worth another try to see if you can still reproduce this crash.

stevengill · 2020-09-03T18:38:44Z

Going to close this issue due to inactivity.

aoberoi added the needs info An issue that is claimed to be a bug and hasn't been reproduced, or otherwise needs more info label Jul 3, 2018

aoberoi removed the needs info An issue that is claimed to be a bug and hasn't been reproduced, or otherwise needs more info label Jul 24, 2018

aoberoi added the needs info An issue that is claimed to be a bug and hasn't been reproduced, or otherwise needs more info label Aug 2, 2018

stevengill closed this as completed Sep 3, 2020

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Attempting to use the SDK when Slack is down will crash the Node process #586

Attempting to use the SDK when Slack is down will crash the Node process #586

wilg commented Jun 27, 2018

aoberoi commented Jun 28, 2018

wilg commented Jun 28, 2018

aoberoi commented Jun 28, 2018

lukechilds commented Jul 3, 2018

aoberoi commented Jul 3, 2018

lukechilds commented Jul 3, 2018

lukechilds commented Jul 3, 2018

aoberoi commented Jul 3, 2018

wilg commented Jul 3, 2018

aoberoi commented Jul 23, 2018

wilg commented Jul 24, 2018

aoberoi commented Jul 24, 2018

wilg commented Jul 24, 2018 •

edited

Loading

wilg commented Jul 24, 2018

aoberoi commented Jul 24, 2018

Ismaw34 commented Jul 30, 2018

aoberoi commented Jul 31, 2018

aoberoi commented Jul 31, 2018

wilg commented Jul 31, 2018

aoberoi commented Sep 13, 2018

stevengill commented Sep 3, 2020

Attempting to use the SDK when Slack is down will crash the Node process #586

Attempting to use the SDK when Slack is down will crash the Node process #586

Comments

wilg commented Jun 27, 2018

Description

What type of issue is this? (place an x in one of the [ ])

Requirements (place an x in each of the [ ])

Bug Report

Reproducible in:

Steps to reproduce:

Expected result:

Actual result:

Attachments:

aoberoi commented Jun 28, 2018

wilg commented Jun 28, 2018

aoberoi commented Jun 28, 2018

lukechilds commented Jul 3, 2018

aoberoi commented Jul 3, 2018

lukechilds commented Jul 3, 2018

lukechilds commented Jul 3, 2018

aoberoi commented Jul 3, 2018

wilg commented Jul 3, 2018

aoberoi commented Jul 23, 2018

wilg commented Jul 24, 2018

aoberoi commented Jul 24, 2018

wilg commented Jul 24, 2018 • edited Loading

wilg commented Jul 24, 2018

aoberoi commented Jul 24, 2018

Ismaw34 commented Jul 30, 2018

aoberoi commented Jul 31, 2018

aoberoi commented Jul 31, 2018

wilg commented Jul 31, 2018

aoberoi commented Sep 13, 2018

stevengill commented Sep 3, 2020

What type of issue is this? (place an `x` in one of the `[ ]`)

Requirements (place an `x` in each of the `[ ]`)

wilg commented Jul 24, 2018 •

edited

Loading