kiliman / remix-express-vite-plugin

This package includes a Vite plugin to use in your Remix app. It configures an Express server for both development and production using TypeScript.
118 stars 6 forks source link

` Cannot read properties of undefined (reading 'pathname')` when using https #15

Closed ZipBrandon closed 4 months ago

ZipBrandon commented 4 months ago

I am trying to use this with an SSL certificate period and it crashes immediately on the request. It works fine when not using 443 and certificate.

my vite.config.ts

    server: {
        port: 443,
        host: "0.0.0.0",
        https: {
            key: "./certs/privkey.pem",
            cert: "./certs/fullchain.pem",
        },
    },
TypeError: Cannot read properties of undefined (reading 'pathname')
    at next (file:///Users/lazerskates/DEVELOPMENT/frontend-express/node_modules/vite/dist/node/chunks/dep-cNe07EU9.js:42861:31)
    at /Users/lazerskates/DEVELOPMENT/frontend-express/node_modules/express/lib/router/index.js:646:15
    at next (/Users/lazerskates/DEVELOPMENT/frontend-express/node_modules/express/lib/router/index.js:216:14)
    at Layer.handle [as handle_request] (/Users/lazerskates/DEVELOPMENT/frontend-express/node_modules/express/lib/router/layer.js:97:5)
    at trim_prefix (/Users/lazerskates/DEVELOPMENT/frontend-express/node_modules/express/lib/router/index.js:328:13)
    at /Users/lazerskates/DEVELOPMENT/frontend-express/node_modules/express/lib/router/index.js:286:9
    at Function.process_params (/Users/lazerskates/DEVELOPMENT/frontend-express/node_modules/express/lib/router/index.js:346:12)
    at next (/Users/lazerskates/DEVELOPMENT/frontend-express/node_modules/express/lib/router/index.js:280:10)
    at Function.handle (/Users/lazerskates/DEVELOPMENT/frontend-express/node_modules/express/lib/router/index.js:175:3)
    at Function.handle (/Users/lazerskates/DEVELOPMENT/frontend-express/node_modules/express/lib/application.js:181:10)
    at file:///Users/lazerskates/DEVELOPMENT/frontend-express/node_modules/remix-express-dev-server/dist/index.js:52:25
kiliman commented 4 months ago

Ok, thanks. I'll take a look. I'm pretty sure I tested this scenario.

Just remember, anything related to development should be configured in vite.config.ts, so the server configuration there is for development only.

For the production server (if you want HTTPS or HTTP/2 on production), you use the createServer option in createExpressApp.

I'll double-check that you can use HTTPS on the dev server.

ZipBrandon commented 4 months ago

Correct, currently I am just trying to use the dev server and experiencing this issue. Thank you for clarifying, too.

gustavopch commented 4 months ago

In my custom Express app (not using this package yet), I had to configure HTTPS in two places:

...

const protocol =
  process.env.SSL_KEY_PATH && process.env.SSL_CERT_PATH
    ? 'https'
    : 'http'

const viteDevServer =
  process.env.NODE_ENV === 'production'
    ? undefined
    : await import('vite').then(vite =>
        vite.createServer({
          server: {
            middlewareMode: true,
            https:
              protocol === 'https'
                ? {
                    key: readFileSync(process.env.SSL_KEY_PATH),
                    cert: readFileSync(process.env.SSL_CERT_PATH),
                  }
                : undefined,
          },
        }),
      )

...

const server =
  protocol === 'https'
    ? createSecureServer(
        {
          key: readFileSync(process.env.SSL_KEY_PATH),
          cert: readFileSync(process.env.SSL_CERT_PATH),
        },
        app,
      )
    : app

server.listen(port, () => {
  console.log(`\nšŸŒ Listening at: ${protocol}://localhost:${port}`)
})

I believe we'd need to be able to pass that config to vite.createServer here: https://github.com/kiliman/remix-express-vite-plugin/blob/8bd9909caa2059f3af9f8e8c618c55263eade2f3/packages/remix-create-express-app/src/index.ts#L148-L157

ZipBrandon commented 4 months ago

@gustavopch That's curious. I don't have to do that with vite in my regular app. When Vite was released in November I have only had to define the cert and key in the vite.config.ts.

My server.ts defines the dev server as:

const viteDevServer =
    process.env.NODE_ENV === "production"
        ? undefined
        : await import("vite").then(({ createServer }) =>
                createServer({
                    server: {
                        middlewareMode: true,
                    },
                }),
            );

and creates the request handler with:

build: viteDevServer
            ? () => viteDevServer.ssrLoadModule("virtual:remix/server-build")
            : await import("../build/deployment/server/index.js"),
kiliman commented 4 months ago

I've been tracing the error. Yes, to define a secure Vite dev server, you only need to configure server.https with your cert. Vite will launch a secure server.

My expressDevServer plugin loads the Remix/Express app and then sends the request to it. It works perfectly with standard HTTP, but for some reason, when it gets to the expressInit handler (express internals), it loses req.url (undefined), and that's where the error Cannot read properties of undefined (reading 'pathname') comes from.

Anyway, I'll keep digging.

gustavopch commented 4 months ago

@kiliman It would be great if you could add an example to this repo with the HTTPS configuration when you figure it out.

I'm getting ERR_SSL_VERSION_OR_CIPHER_MISMATCH. šŸ˜• Nevermind, I wasn't passing the right values to server.https. Now I'm having the pathname error too.

kiliman commented 4 months ago

Yes, this is my vite.config

export default defineConfig({
  build: {
    target: 'esnext',
  },
  server: {
    https: {
      key: fs.readFileSync(process.cwd() + '/server/localhost-key.pem'),
      cert: fs.readFileSync(process.cwd() + '/server/localhost-cert.pem'),
    },
  },
  plugins: [
    expressDevServer(),
    envOnly(),
    remix({
      future: { unstable_singleFetch: true },
    }),
    tsconfigPaths(),
  ],
})
gustavopch commented 4 months ago

@kiliman FWIW, req.url turns to undefined exactly after setPrototypeOf(req, app.request) at expressInit (<omitted>/node_modules/express/lib/middleware/init.js:43:5).

gustavopch commented 4 months ago

Adding console.log(Object.getOwnPropertyDescriptor(req, 'url')) in the line before setPrototypeOf(req, app.request):

gustavopch commented 4 months ago

Adding console.log(Object.getOwnPropertyDescriptors(req)) before (left) and after (right) setPrototypeOf(req, app.request):

When using HTTP:

image

When using HTTPS:

image
gustavopch commented 4 months ago

OK, that's as far as I could get. Not sure where to go now. This is the analysis from GPT-4o after reading all the text and these last 2 images:

The issue seems to be rooted in the differences between HTTP/1.x and HTTP/2 request handling in Node.js and Express. Specifically, the req object for HTTP/2 (Http2ServerRequest) behaves differently from the req object for HTTP/1.x (IncomingMessage), and this difference is causing issues when the prototype is set.

Key Observations

  1. HTTP/1.x (IncomingMessage) vs. HTTP/2 (Http2ServerRequest):

    • HTTP/1.x uses IncomingMessage for the req object.
    • HTTP/2 uses Http2ServerRequest for the req object.
  2. Prototype Chain Differences:

    • The setPrototypeOf call affects the req object differently based on its type, leading to properties like url becoming undefined.

Potential Root Cause

The root cause is likely due to the fact that Http2ServerRequest and IncomingMessage have different internal structures and prototype chains. When setPrototypeOf is called, it disrupts the expected structure of the Http2ServerRequest object, leading to properties becoming undefined.

Then it gives some suggestions that don't seem very useful. I'll stick with HTTP for now. Hope these clues lead into the right direction.

kiliman commented 4 months ago

Thanks for digging in. That reminded me that I actually do support that. You have to also configure the express app to handle http2 requests:

// app/entry.server.tsx
import express from 'express'
import http2Express from 'http2-express-bridge'
import fs from 'node:fs'
import http2 from 'node:http2'

export const app = createExpressApp({
  getExpress: () => {
    // create a custom express app, needed for HTTP/2
    return http2Express(express)
  },
  createServer: app => {
    // create a custom server for production
    // use Vite config `server` to customize the dev server
    return http2.createSecureServer(
      {
        key: fs.readFileSync(process.cwd() + '/server/localhost-key.pem'),
        cert: fs.readFileSync(process.cwd() + '/server/localhost-cert.pem'),
      },
      app,
    )
  },
})
kiliman commented 4 months ago

Note: The createServer() is only used for production. You configure the Vite dev server via vite.config.ts

kiliman commented 4 months ago

This should be fixed following the instructions above.

ZipBrandon commented 4 months ago

@kiliman Is this a me thing? I made the changes and it proceeded further. It now crashes on compression. I'm attempting to troubleshoot on my end.

8:01:17 AM [vite] Internal server error: this._implicitHeader is not a function
      at Http2ServerResponse.write (/Users/lazerskates/.../fvn-remix/node_modules/compression/index.js:84:14)
      at writeReadableStreamToWritable (/Users/lazerskates/.../fvn-remix/node_modules/@remix-run/node/dist/stream.js:30:16)
      at processTicksAndRejections (node:internal/process/task_queues:95:5)
      at sendRemixResponse (file:///Users/lazerskates/.../fvn-remix/node_modules/remix-create-express-app/dist/remix.js:92:5)
      at file:///Users/lazerskates/DEVELOPMENT/.../fvn-remix/node_modules/remix-create-express-app/dist/remix.js:17:7

update: I can confirm that using the instructions that it will work with the example app in this repo. I'm seeing what is askew in my simple app to cause my above issue.


update: it expects the configure key to be defined in the createExpressApp even if it does nothing. @kiliman should I make a new issue or could this ride under here?

    configure: (app) => {   // <------- removing this definition would result in [vite] Internal server error: this._implicitHeader is not a function
        // customize your express app with additional middleware
        // app.use(morgan("tiny"));
    },