inngest / inngest-js

The developer platform for easily building reliable workflows with zero infrastructure for TypeScript & JavaScript
https://www.inngest.com/
GNU General Public License v3.0
440 stars 43 forks source link

[BUG] Inngest with cloudflare remote bindings #664

Closed BernardoSM closed 5 months ago

BernardoSM commented 5 months ago

Describe the bug Is not possible to use inngest locally with a cloudflare project using remote bindings.

To Reproduce Steps to reproduce the behavior:

  1. Clone this repo: https://github.com/inngest/inngest-js/tree/main/examples/framework-hono
  2. Update package.json with the code below
    {
    "scripts": {
    "dev": "wrangler dev --port 3000 src/index.ts --remote",
    "deploy": "wrangler deploy --minify src/index.ts"
    },
    "dependencies": {
    "hono": "^4.2.7",
    "inngest": "^3.0.0"
    },
    "devDependencies": {
    "@cloudflare/workers-types": "^4.20240403.0",
    "wrangler": "^3.47.0"
    }
    }
  3. Run the command npm run dev
  4. Run the command npx inngest-cli dev -u http://localhost:3000/api/inngest --no-discovery

Expected behavior Inngest should connect with cloudflare worker with --remote flag.

Screenshots / Stack trace dump Instead of connecting with the app, it shows the error:

Error: Expected server kind cloud, got dev

CleanShot 2024-06-17 at 11 50 01

System info

linear[bot] commented 5 months ago

INN-3180 Inngest with cloudflare remote bindings

lorenzodejong commented 5 months ago

Adding some info from an issue on inngest-js.

First of all, i think it would be awesome to support this as a feature. Using this approach you can have a local development experience whilst you execute your code in a cloud environment, with all the capabilities of the provided Cloudflare bindings.

I tried to circumvent this by explicitly setting isDev to true on the Inngest client. This does allow registration of the instance in the Inngest dev server, however any invoked event will cause an error from Cloudflare: Error 1003 : Direct IP access not allowed. It seems like the Inngest dev server is explicitly trying to access a port on the host, which is not allowed from Cloudflare.

jpwilliams commented 5 months ago

Thanks for the report, @BernardoSM!

Is Wrangler with --remote able to access localhost? 🤔

With your error, @lorenzodejong, I'd guess that that's an error from the SDK trying to access 127.0.0.1; it seems any IP access is not allowed: Known issues: Fetch to IP addresses.

With that--and assuming that accessing localhost has the same issue--you'd likely have to use something like ngrok or localtunnel to expose the Dev Server to the remote worker. Something like:

# CLI
npx inngest-cli dev -u http://localhost:3000/api/inngest --no-discovery
ngrok http 8288
# wrangler.toml
[vars]
INNGEST_DEV = "https://YOUR_ID.ngrok.app"
INNGEST_SERVE_HOST = "http://localhost:3000" # the remote worker may set this to a public URL, so override it here

That should enable the remote worker to connect if localhost is being an issue. I've added this to inngest/website#817, though am keen to see if another route is preferable.

lorenzodejong commented 5 months ago

@jpwilliams that would actually be a fine workaround, will try that out once i'll get the chance.

Isn't the SDK trying to access the IP of the Cloudflare Worker instance? At least that's what I expected based on the 1003 error that was provided from Cloudflare.

jpwilliams commented 5 months ago

@lorenzodejong The sync flow is a little complex in order to also support apps registering themselves via a simple PUT. We want to simplify that a bit in the future, but for now:

sequenceDiagram

participant IDS as Inngest Dev Server
participant W as Wrangler Tunnel
participant CF as Cloudflare Remote Worker
participant CFG as Cloudflare Gateway

IDS->>+W: PUT http://localhost:3000/api/inngest
W->>+CF: PUT https://remote-worker/api/inngest
CF->>+CFG: POST http://127.0.0.1:8288/fn/register
CFG-->>-CF: Error 1003: No Direct IP access!
CF-->>-W: Error 1003: No Direct IP access!
W-->>-IDS: Error 1003: No Direct IP access!

With another tunnel set up for the Dev Server:

sequenceDiagram

participant N as ngrok
participant IDS as Inngest Dev Server
participant W as Wrangler Tunnel
participant CF as Cloudflare Remote Worker
participant CFG as Cloudflare Gateway

IDS->>+W: PUT http://localhost:3000/api/inngest
W->>+CF: PUT https://remote-worker/api/inngest
CF->>+CFG: POST https://ngrok-url/fn/register
CFG->>+N: POST https://ngrok-url/fn/register
N->>+IDS: POST http://localhost:8288/fn/register
IDS-->>-N: 200 OK
N-->>-CFG: 200 OK
CFG-->>-CF: 200 OK
CF-->>-W: 200 OK
W-->>-IDS: 200 OK

😮‍💨

The INNGEST_SERVE_HOST is also needed as otherwise I'm guessing the remote worker would identify itself as its actual address (e.g. https://some-remote-worker/api/inngest), meaning the Dev Server would attempt to access it at that address and bypass Wrangler's tunnel entirely.

I don't think that would cause any problems, though it seems a waste to not use their tunnel. 😅

lorenzodejong commented 5 months ago

Thanks for the detailed answer, i understand the issue concerning the self-registration of the app.

Another alternative to Ngrok/localtunnel is Cloudflare tunnel. Which could potentially be good to mention in the documentation as well, as it's a solution in the same ecosystem 😄

lorenzodejong commented 5 months ago

Another question I currently have related to Cloudflare bindings is how to handle environment variables.

From Cloudflare environment variables are explicitly passed to the handler. Getting an environment variable from process.env is not possible.

With the current setup of Cloudflare workers we don't get a reference to the env object passed from the Workers' handler.

Not sure if this is an oversight from my side, or if this is actually lacking in the current recommended setup.

jpwilliams commented 5 months ago

@lorenzodejong: Another alternative to Ngrok/localtunnel is Cloudflare tunnel. Which could potentially be good to mention in the documentation as well, as it's a solution in the same ecosystem 😄

Ah, awesome! I'll add that to our docs too!

@lorenzodejong: With the current setup of Cloudflare workers we don't get a reference to the env object passed from the Workers' handler.

Internally, the "inngest/cloudflare" entrypoint accesses env, but aye, it doesn't expose it to you. You can write a small piece of middleware that adds it to the arguments for every function. Something like:

// The usual manual TS for Cloudflare's env
export interface Env {
  API_HOST: string;
}

// Create a middleware that will parse arguments passed to the
// serve handler, grab `env`, and add it to our function input.
const cloudflareMiddleware = new InngestMiddleware({
  name: "Cloudflare Env",
  init: () => {
    return {
      onFunctionRun: ({ reqArgs }) => {
        const [,env] = reqArgs as [unknown, Env];

        return {
          transformInput: () => {
            // add `env` to function args
            return { ctx: { env } };
          },
        };
      },
    };
  },
});

// Create our client with the middleware
const inngest = new Inngest({
  id: "my-app",
  middleware: [cloudflareMiddleware],
});

// `env` is now available in every function
inngest.createFunction(
  { id: "my-fn" },
  { event: "some.event" },
  async ({ event, step, env }) => {
    // `env` is typed and available
  },
);

We see a lot of folks using this to use the environment and anything that needs it like new Ai(env.AI).

A middleware from us that does this might be reasonable...

jpwilliams commented 5 months ago

Going to close this as the workaround seems reasonable for now given Cloudflare's lack of access to localhost when deployed with --remote.

amlcodes commented 3 months ago

I cannot get env.AI working in any case

jpwilliams commented 3 months ago

Is this example not working for you, @amlcodes?

https://www.inngest.com/docs/reference/middleware/examples#cloudflare-workers-ai

amlcodes commented 3 months ago

sorry for the lack of update, @jpwilliams

the example uses the deprecated Ai package, and manipulates the env* when accessed as shown for some reason. I have to pull it straight from the request context.

const [ctx, env] = reqArgs as [Request, Env]; 
// const [, env] = reqArgs as [Request, Env]; 

 return {
                    transformInput: () => {
                        // add `env` to function args
                        return {
                            ctx,
                            // ctx: {
                            // env,
                            // },
                        };
                    },
                };
jpwilliams commented 3 months ago

@amlcodes Ah, fun! Looks like it's just env.AI now by default? Is that right?

# wranger.toml
[ai]
binding = "AI"
import { InngestMiddleware } from "inngest";

interface Env {
  // If you set another name in wrangler.toml as the value for 'binding',
  // replace "AI" with the variable name you defined.
  AI: Ai;
}

export const cloudflareMiddleware = new InngestMiddleware({
  name: "Inngest: Workers AI",
  init: () => {
    return {
      onFunctionRun: ({ reqArgs }) => {
        const [, env] = reqArgs as [Request, Env];
        const ai = env.AI;

        return {
          transformInput: () => {
            return { ctx: { ai } };
          },
        };
      },
    };
  },
});
linear[bot] commented 3 months ago

INN-3352 [BUG] Inngest with cloudflare remote bindings

amlcodes commented 3 months ago

@amlcodes Ah, fun! Looks like it's just env.AI now by default? Is that right?

# wranger.toml
[ai]
binding = "AI"
import { InngestMiddleware } from "inngest";

interface Env {
  // If you set another name in wrangler.toml as the value for 'binding',
  // replace "AI" with the variable name you defined.
  AI: Ai;
}

export const cloudflareMiddleware = new InngestMiddleware({
  name: "Inngest: Workers AI",
  init: () => {
    return {
      onFunctionRun: ({ reqArgs }) => {
        const [, env] = reqArgs as [Request, Env];
        const ai = env.AI;

        return {
          transformInput: () => {
            return { ctx: { ai } };
          },
        };
      },
    };
  },
});

yes, but for some reason, accessing


const [, env] = reqArgs as [Request, Env];
const ai = env.AI;

instead of

        const [ctx, env] = reqArgs as [Request, Env]; 
        const ai = ctx.env.AI

doesn't give me the binding. the destructured [env] seems to be manipulating the ctx.env. ctx.env works so far though.

jpwilliams commented 3 months ago

Oh, fun!

Thanks for this, @amlcodes - I'll update our example on the site! 🙌