Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Support for cloudflare workers #1335

Closed
5 of 17 tasks
roopakv opened this issue Sep 14, 2021 · 10 comments
Closed
5 of 17 tasks

Support for cloudflare workers #1335

roopakv opened this issue Sep 14, 2021 · 10 comments
Labels
auto-triage-skip discussion M-T: An issue where more input is needed to reach a decision enhancement M-T: A feature request for new functionality pkg:web-api applies to `@slack/web-api`

Comments

@roopakv
Copy link

roopakv commented Sep 14, 2021

Description

the web-api npm package is not usable on Cloudflare workers since it uses axios. If this SDK were to consider using something like isomorphic-fetch, one could use this package in workers too.

I am happy to make a contribution but want input before I start working on this

What type of issue is this?

  • bug
  • enhancement (feature request)
  • question
  • documentation related
  • testing related
  • discussion

Requirements

  • 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.

Packages:

Select all that apply:

  • @slack/web-api
  • @slack/events-api
  • @slack/interactive-messages
  • @slack/rtm-api
  • @slack/webhooks
  • @slack/oauth
  • @slack/socket-mode
  • I don't know
@srajiang srajiang added enhancement M-T: A feature request for new functionality pkg:web-api applies to `@slack/web-api` labels Sep 14, 2021
@srajiang
Copy link
Member

srajiang commented Sep 14, 2021

@roopakv - Thanks for writing in with this suggestion and for your interest in working on this! It's kicked up a healthy discussion amongst the maintainers, and I will let others (@filmaj @seratch @stevengill) chime in with their input as well.

In general, the team isn't planning on working on enhancing of the web-api package to enable developers to use a different HTTP client, however we would be happy to work with you to review this if it's something you're interested in working on. [Edit] Or if you're thinking of creating your own solution to cover your use case and you make your work public, we are happy to share the repo out with the developer communities.

@srajiang srajiang added the discussion M-T: An issue where more input is needed to reach a decision label Sep 14, 2021
@roopakv
Copy link
Author

roopakv commented Sep 15, 2021

@srajiang if i were to send PRs up moving from axios to something else would your team be open to reviewing and getting it in?

If not I would probably look into making my own solution based off of a fork of this repo :)

@filmaj
Copy link
Contributor

filmaj commented Sep 15, 2021

Hey @roopakv,

If you want to attempt to replace axios, please feel free, though I have a feeling it is a big effort. There are many critical aspects of this package to the variety of different Slack customers, so all the various options (like proxying, file uploading, rate limit handling, request concurrency, custom TLS configurations, custom API URL) would have to be honoured and the test suites would need to pass.

The web-api package is also the foundation for many other Slack SDKs, so the test suites (including the integration tests in the root of this repo) would need to pass. Some of these require Slack Enterprise org/workspace, which, I assume, you would not have access to and would need to be coordinated with someone from Slack.

If you do attempt to take this on, may I suggest you begin working in your fork on a branch, and before sending a PR to us, you ping us (in this issue is fine) to let us know when you think it is in a workable state before opening a PR? This way I can provide some feedback before we go through a full review and back and forth process.

@filmaj
Copy link
Contributor

filmaj commented Oct 25, 2021

BTW, in case this is helpful, I was able to get an ES Modules-based version of @slack/web-api compatible with deno by making an esbuild- based build of the project, with no code changes. Perhaps that could be useful for someone wanting to get this project working on Cloudflare Workers.

Roughly:

  • use esbuild
  • use the browser field in package.json to stub in certain npm packages to replace built-in node ones with ones built for the browser, i.e. replacing path with the path-browserify npm module. The list of native node dependencies @slack/web-api relies on is path, os, querystring and process.
  • (optional) Depending on your target runtime, perhaps polyfilling the Buffer global is not necessary, but if so, use the experimental Buffer compatibility package available in unstable deno together with esbuild's --define and --inject flags to stub in the Buffer global to be available in deno.
  • use an XMLHttpRequest polyfill (there are many available in npm and deno.land) in deno since deno only supports fetch, but axios only supports either node's HTTP API (which deno has no compatibility for) or XHR. Note: there is a concept called "adapters" in axios, which provides a clean API to implement your own network layer in axios - this is a solid backup option and should be flexible to enable running in any runtime)

Some more specifics on how a deno-compatible build was compiled:

Esbuild command:

esbuild --bundle --define:process.cwd=String --define:process.version='"v14.0.0"' --define:Buffer=dummy_buffer --inject:./buffer-shim.js --target=esnext --format=esm --outdir=./dist-esm  src/index.ts

Some package.json changes were needed, like adding the browser field, as well as add browser-compatible versions of certain native node APIs, e.g. path-browserify and os-browserify:

  "browser": {
    "os": "os-browserify",
    "path": "path-browserify",
    "querystring": "./qs-shim.js"
  }

The buffer-shim.js referenced in the esbuild command is:

import { Buffer } from "https://deno.land/std@0.112.0/node/buffer.ts"

export let dummy_buffer = Buffer

The qs-shim.js referenced in the esbuild command is:

export * from "https://deno.land/std@0.112.0/node/querystring.ts"

NB: could probably use the same shim-in-deno-compatibility-node-modules approach for path, process and os, too in case we don't want to add more dependencies via browserify packages.

@github-actions
Copy link

github-actions bot commented Dec 5, 2021

👋 It looks like this issue has been open for 30 days with no activity. We'll mark this as stale for now, and wait 10 days for an update or for further comment before closing this issue out.

@jimmywarting
Copy link
Contributor

jimmywarting commented Jun 7, 2022

👍 on removing axios and replacing it with fetch (most preferable the new global built in fetch from node v18) so it dose not have any dependencies

And then also ditching form-data for the newer spec'ed FormData built in also

if ppl can't update to node v18 then i think they could add a polyfill themself

maybe add:

"engines": {
    "node": ">=18.0.0"
}

@nbolton
Copy link

nbolton commented Jul 17, 2023

Judging by this ticket, looks like Slack API is not supported on Cloudflare Workers. Here's the error I get:

X [ERROR] Could not resolve "querystring"

    ../../node_modules/@slack/web-api/dist/WebClient.js:49:30:
      49 │ const querystring_1 = require("querystring");
         ╵                               ~~~~~~~~~~~~~

  The package "querystring" wasn't found on the file system but is built into node.
  Add "node_compat = true" to your wrangler.toml file to enable Node.js compatibility.


X [ERROR] Could not resolve "path"

    ../../node_modules/@slack/web-api/dist/WebClient.js:50:23:
      50 │ const path_1 = require("path");
         ╵                        ~~~~~~

  The package "path" wasn't found on the file system but is built into node.
  Add "node_compat = true" to your wrangler.toml file to enable Node.js compatibility.


X [ERROR] Could not resolve "zlib"

    ../../node_modules/@slack/web-api/dist/WebClient.js:57:39:
      57 │ const zlib_1 = __importDefault(require("zlib"));
         ╵                                        ~~~~~~

  The package "zlib" wasn't found on the file system but is built into node.
  Add "node_compat = true" to your wrangler.toml file to enable Node.js compatibility.


X [ERROR] Could not resolve "util"

    ../../node_modules/@slack/web-api/dist/WebClient.js:58:23:
      58 │ const util_1 = require("util");
         ╵                        ~~~~~~

  The package "util" wasn't found on the file system but is built into node.
  Add "node_compat = true" to your wrangler.toml file to enable Node.js compatibility.


X [ERROR] Could not resolve "fs"

    ../../node_modules/@slack/web-api/dist/file-upload.js:4:21:
      4 │ const fs_1 = require("fs");
        ╵                      ~~~~

  The package "fs" wasn't found on the file system but is built into node.
  Add "node_compat = true" to your wrangler.toml file to enable Node.js compatibility.


X [ERROR] Could not resolve "stream"

    ../../node_modules/@slack/web-api/dist/file-upload.js:5:25:
      5 │ const stream_1 = require("stream");
        ╵                          ~~~~~~~~

  The package "stream" wasn't found on the file system but is built into node.
  Add "node_compat = true" to your wrangler.toml file to enable Node.js compatibility.


X [ERROR] Could not resolve "os"

    ../../node_modules/@slack/web-api/dist/instrument.js:27:32:
      27 │ const os = __importStar(require("os"));
         ╵                                 ~~~~

  The package "os" wasn't found on the file system but is built into node.
  Add "node_compat = true" to your wrangler.toml file to enable Node.js compatibility.


X [ERROR] Could not resolve "path"

    ../../node_modules/@slack/web-api/dist/instrument.js:28:23:
      28 │ const path_1 = require("path");
         ╵                        ~~~~~~

  The package "path" wasn't found on the file system but is built into node.
  Add "node_compat = true" to your wrangler.toml file to enable Node.js compatibility.


X [ERROR] Build failed with 8 errors:

  ../../node_modules/@slack/web-api/dist/WebClient.js:49:30: ERROR: Could not resolve "querystring"
  ../../node_modules/@slack/web-api/dist/WebClient.js:50:23: ERROR: Could not resolve "path"
  ../../node_modules/@slack/web-api/dist/WebClient.js:57:39: ERROR: Could not resolve "zlib"
  ../../node_modules/@slack/web-api/dist/WebClient.js:58:23: ERROR: Could not resolve "util"
  ../../node_modules/@slack/web-api/dist/file-upload.js:4:21: ERROR: Could not resolve "fs"
  ...

@nbolton
Copy link

nbolton commented Jul 17, 2023

Actually, I just spotted this in the error message!

Add "node_compat = true" to your wrangler.toml file to enable Node.js compatibility.

After adding this, the dev server starts, but then as expected, I get the error from the axios dep:

[WARN]  web-api:WebClient:0 http request failed adapter is not a function
[mf:err] TypeError: adapter is not a function
    at dispatchRequest (\path\to\project\node_modules\axios\lib\core\dispatchRequest.js:58:10)  
    at Axios.request (\path\to\project\node_modules\axios\lib\core\Axios.js:109:15)
    at Axios.httpMethod [as post] (\path\to\project\node_modules\axios\lib\core\Axios.js:144:19)
    at Function.wrap2 [as post] (\path\to\project\node_modules\axios\lib\helpers\bind.js:9:15)
    at null.<anonymous> (\path\to\project\node_modules\@slack\web-api\src\WebClient.ts:560:43)
    at run (\path\to\project\node_modules\p-queue\dist\index.js:157:104)
    at PQueue._tryToStartAnother (\path\to\project\node_modules\p-queue\dist\index.js:105:17)
    at null.<anonymous> (\path\to\project\node_modules\p-queue\dist\index.js:171:18)
    at [object Object]
    at PQueue.add (\path\to\project\node_modules\p-queue\dist\index.js:152:16)

Possibly useful: https://stackoverflow.com/a/70206333/47775

Axios accepts an adapter field in its configuration, so you can pass a fetch adapter like axios-fetch-adapter and probably, you'll need to add regenerator-runtime too, so in your background.js file:

import "regenerator-runtime/runtime.js";

@seratch
Copy link
Member

seratch commented Jul 18, 2023

👋 I understand that many of you would like to have a 1st party / official SDK for Cloudflare Workers. However, the current SDK only supports Node.js, and our team has no plans to enhance it to support other types of runtimes, such as non-Node.js edge functions at this moment.

Recently, I began a hobby weekend project, a tool for Cloudflare and Vercel. If you’re open to using an unofficial library, my project could be useful for you. For more details, please refer to #1603 (comment)

mlbrgl added a commit to owid/owid-grapher that referenced this issue Jan 11, 2024
Add Sentry logging for /donation/* Cloudflare Pages functions routes.

At the time of writing, neither [bugsnag](bugsnag/bugsnag-js#637) nor [slack](slackapi/node-slack-sdk#1335) have official Cloudflare worker integrations.

Given the criticality of these routes, I favoured reliability and introduced a new logging app which has official Cloudflare support: https://developers.cloudflare.com/pages/functions/plugins/sentry/

We should switch to bugsnag whenever official support is available.

Testing:
- (ENV and SENTRY_DSN secrets are already defined for the preview environment in Cloudflare's "owid" project)
- temporarily disable the [Cloudflare Access rule](https://one.dash.cloudflare.com/078fcdfed9955087315dd86792e71a7e/access/apps/edit/d8c658c3-fd20-477e-ac20-e7ed7fd656de?tab=overview) by setting its  subdomain to `temp`
- send an empty JSON to https://donate.owid.pages.dev/donation/donate

-> an error is logged in sentry and an alert email is sent to [TBD]@ourworldindata.org (this will be configured when the use of Sentry is confirmed). Slack integration is only available on paid plans.
@filmaj
Copy link
Contributor

filmaj commented Jan 26, 2024

With @seratch providing an alternative project, going to close this issue, as web-api has no plans to officially support a runtime beyond node.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
auto-triage-skip discussion M-T: An issue where more input is needed to reach a decision enhancement M-T: A feature request for new functionality pkg:web-api applies to `@slack/web-api`
Projects
None yet
Development

No branches or pull requests

6 participants