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

connect-web requests fail in next.js middleware #749

Closed
aplr opened this issue Aug 9, 2023 · 6 comments
Closed

connect-web requests fail in next.js middleware #749

aplr opened this issue Aug 9, 2023 · 6 comments
Labels
bug Something isn't working

Comments

@aplr
Copy link

aplr commented Aug 9, 2023

Describe the bug

I try to use connect-web in the Next.js middleware, as Node APIs are not available, thus connect-node does not work.

I expect it to work, as it's just using the fetch api, and Next.js is running on V8, just as connect-web in the browser.

I expect it to work, as the edge runtime seems to support all necessary features required by connect-web.

However, requests fail with:

- error Error [TypeError]: Cannot read properties of undefined (reading 'flags')
    at setRemoved (file:///app/node_modules/next/dist/compiled/edge-runtime/index.js:1:970334)
    at removeListenerAt (file:///app/node_modules/next/dist/compiled/edge-runtime/index.js:1:970334)
    at EventTarget.dispatchEvent (file:///app/node_modules/next/dist/compiled/edge-runtime/index.js:1:970334)
    at abortSignalAbort (file:///app/node_modules/next/dist/compiled/edge-runtime/index.js:1:970334)
    at AbortController.abort (file:///app/node_modules/next/dist/compiled/edge-runtime/index.js:1:970334)
    at done (webpack-internal:///(middleware)/./node_modules/@bufbuild/connect/dist/esm/protocol/run-call.js:103:24)
    at eval (webpack-internal:///(middleware)/./node_modules/@bufbuild/connect/dist/esm/protocol/run-call.js:33:9)
...

Also saw #550, which reports an issue w/ cloudflare workers & the AbortController, which also is a similar configuration, however not consuming but providing connect services. does not seem to be related

To Reproduce

  1. Clone reproduction repo: aplr/next_connect_middleware_issue
  2. Install dependencies: npm install
  3. Start server: npm run dev
  4. Visit http://localhost:3000 in browser
  5. Look at the logs in the terminal

Expected behavior

I expect the middleware to send the request using connect-web's connect transport, which uses only APIs available in browsers. Therefore I expect it to work also in vercel's edge runtime, which is running a browser-like environment.

Actual behavior

The following error is thrown:

- error Error [TypeError]: Cannot read properties of undefined (reading 'flags')
    at setRemoved (file:///app/node_modules/next/dist/compiled/edge-runtime/index.js:1:970334)
    at removeListenerAt (file:///app/node_modules/next/dist/compiled/edge-runtime/index.js:1:970334)
    at EventTarget.dispatchEvent (file:///app/node_modules/next/dist/compiled/edge-runtime/index.js:1:970334)
    at abortSignalAbort (file:///app/node_modules/next/dist/compiled/edge-runtime/index.js:1:970334)
    at AbortController.abort (file:///app/node_modules/next/dist/compiled/edge-runtime/index.js:1:970334)
    at done (webpack-internal:///(middleware)/./node_modules/@bufbuild/connect/dist/esm/protocol/run-call.js:103:24)
    at eval (webpack-internal:///(middleware)/./node_modules/@bufbuild/connect/dist/esm/protocol/run-call.js:33:9)
    at process.processTicksAndRejections (node:internal/process/task_queues:95:5)
    at async Object.unary (webpack-internal:///(middleware)/./node_modules/@bufbuild/connect-web/dist/esm/connect-transport.js:89:20)
    at async Object.eval [as say] (webpack-internal:///(middleware)/./node_modules/@bufbuild/connect/dist/esm/promise-client.js:132:26)
    at async Object.middleware [as handler] (webpack-internal:///(middleware)/./middleware.ts:18:26)
    at async adapter (webpack-internal:///(middleware)/./node_modules/next/dist/esm/server/web/adapter.js:170:20)
    at async runWithTaggedErrors (file:///app/node_modules/next/dist/server/web/sandbox/sandbox.js:98:24)
    at async DevServer.runMiddleware (file:///app/node_modules/next/dist/server/next-server.js:1013:24)
    at async DevServer.runMiddleware (file:///app/node_modules/next/dist/server/dev/next-dev-server.js:247:28)
    at async DevServer.handleCatchallMiddlewareRequest (file:///app/node_modules/next/dist/server/next-server.js:1096:22)
    at async DevServer.handleRequestImpl (file:///app/node_modules/next/dist/server/base-server.js:647:32) {
  digest: undefined
}

Environment

  • @bufbuild/connect-web@0.12.0
  • react@18.2.0
  • next@13.4.13
  • node@20.5.0
@aplr aplr added the bug Something isn't working label Aug 9, 2023
@Julianwustl
Copy link

I am also experiencing the same issue. Extremely critical for us, hope this has a high priority.

@timostamm
Copy link
Member

Running Connect on edge platforms such as Vercels edge runtime and Cloudflare workers is an important feature, and we do plan to support them in the future. This is more involved that you might think though. While it's true that Node.js, many browsers, and Vercels Edge runtime use V8 under the hood, V8 is "just" a runtime for ECMAScript. It does not implement any parts of the fetch, streams, or encoding API.

All platforms bring their own implementation of those APIs, which may be incomplete, have bugs, or interpret ambiguous parts of the API spec differently. For example, fetch in Node.js is implemented by undici, and the Vercel Edge runtime is very likely using a different implementation.

When developing locally, you are not actually running code in the Vercel Edge runtime, but in a simulated environment, that may behave slightly differently because it may be running a different implementation of fetch. See the second section in Vercels documentation.

The error you are running into is raised in node_modules/next/dist/compiled/edge-runtime/index.js:1:970334, which seems to be the simulated environment, source code here: https://github.com/vercel/edge-runtime

At this moment, we cannot focus on edge runtime support, but this project being open source, you can help bringing it forward. Finding the location of the error in the source code, and investigating how it is triggered seem to be the logical first steps. It may be an issue we can resolve in connect-es, or it may be an issue that needs a resolution in the edge-runtime package.

@aplr
Copy link
Author

aplr commented Aug 9, 2023

Thanks @timostamm for your extensive comment! Yea I was a bit too unspecific and just wrong here. Just wrote the ticket as I thought there probably might be a quick fix. I was already wondering if it has to do with the edge-runtime and thought about cross-posting. I will try to investigate further, by i.e. running on Vercel itself.

Thanks for clarifying that you won't focus on that right now, I'll investigate further then.

@aplr aplr closed this as completed Aug 9, 2023
@aplr aplr reopened this Aug 9, 2023
@aplr
Copy link
Author

aplr commented Aug 9, 2023

Ok, what I found out:

vercel/edge-runtime is using mysticatea/event-target-shim as a ponyfill.

In its dispatchEvent, it iterates over the listeners, and invokes their callbacks:

https://github.com/mysticatea/event-target-shim/blob/a37993c95ed9cc1d7cb89358ea126ed4f95372e8/src/lib/event-target.ts#L260

It happens to be, that the callback is the onAbort function of your createLinkedAbortController in @bufbuild/connect, which removes the event listeners:

https://github.com/bufbuild/connect-es/blob/76e06a8a5a1ea4403996f6bd7ad3a4840a6780ec/packages/connect/src/protocol/signals.ts#L46-L53

This fiddles with event-target-shim's internal list of listeners and mutates it, removing one event listener. event-target-shim still thinks two event listeners are in the list, therefore iterating over a now deleted index in the array, which returns undefined, which causes the field access to fail with the said error.

The questions now are:

  1. do you think, should removing listeners - like you do it - be supported by event-target-shim, e.g. is it a bug in event-target-shim's way of iterating over the list? (probably yes)
  2. should you rather register a "once" listener in @bufbuild/connect, which will be removed by event-target-shim and other implementations automatically? (see https://github.com/mysticatea/event-target-shim/blob/a37993c95ed9cc1d7cb89358ea126ed4f95372e8/src/lib/event-target.ts#L252-L256)

@aplr
Copy link
Author

aplr commented Aug 25, 2023

This issue was fixed in vercel/edge-runtime by dropping node 14 support & removing the erroneous polyfill (See issue).

The updated vercel/edge-runtime@2.5.0 will land in next.js with version 13.4.20 (See Changelog)

This means, that connect-web will be perfectly usable in vercel's edge functions, as long as you're setting the request's redirect property to follow like that:

import { Interceptor } from "@bufbuild/connect"
import { createConnectTransport } from "@bufbuild/connect-web"

const FollowRedirectsInterceptor: Interceptor = (next) => (req) => {
  req.init.redirect = "follow"
  return next(req)
}

const transport = createConnectTransport({
  baseUrl: "https://example.net/grpc",
  interceptors: [FollowRedirectsInterceptor],
})

Thanks @timostamm for pointing me into the right direction, I'll close this issue.

@aplr aplr closed this as completed Aug 25, 2023
@ddimaria
Copy link

ddimaria commented Sep 5, 2023

Node v16 was problematic for me as well. v18 and v20 work fine though.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

4 participants