search

brillout / goldpage

Page Builder.
Creative Commons Zero v1.0 Universal
57 stars 3 forks source link

Goldpage express handler calls `next()` even when it sends a response #9

Closed chriscalo closed 4 years ago

chriscalo commented 4 years ago

I was noticing the following ERR_HTTP_HEADERS_SENT error for requests when Goldpage handles the response:

App listening on port 8000
Press Ctrl+C to quit.

GET / 304 - - 392.773 ms
Error [ERR_HTTP_HEADERS_SENT]: Cannot set headers after they are sent to the client
    at ServerResponse.setHeader (_http_outgoing.js:470:11)
    at expressInit (/Users/chriscalo/Projects/viaticus/node_modules/express/lib/middleware/init.js:30:42)
    at Layer.handle [as handle_request] (/Users/chriscalo/Projects/viaticus/node_modules/express/lib/router/layer.js:95:5)
    at trim_prefix (/Users/chriscalo/Projects/viaticus/node_modules/express/lib/router/index.js:317:13)
    at /Users/chriscalo/Projects/viaticus/node_modules/express/lib/router/index.js:284:7
    at Function.process_params (/Users/chriscalo/Projects/viaticus/node_modules/express/lib/router/index.js:335:12)
    at next (/Users/chriscalo/Projects/viaticus/node_modules/express/lib/router/index.js:275:10)
    at query (/Users/chriscalo/Projects/viaticus/node_modules/express/lib/middleware/query.js:45:5)
    at Layer.handle [as handle_request] (/Users/chriscalo/Projects/viaticus/node_modules/express/lib/router/layer.js:95:5)
    at trim_prefix (/Users/chriscalo/Projects/viaticus/node_modules/express/lib/router/index.js:317:13)
    at /Users/chriscalo/Projects/viaticus/node_modules/express/lib/router/index.js:284:7
    at Function.process_params (/Users/chriscalo/Projects/viaticus/node_modules/express/lib/router/index.js:335:12)
    at next (/Users/chriscalo/Projects/viaticus/node_modules/express/lib/router/index.js:275:10)
    at Function.handle (/Users/chriscalo/Projects/viaticus/node_modules/express/lib/router/index.js:174:3)
    at Function.handle (/Users/chriscalo/Projects/viaticus/node_modules/express/lib/application.js:174:10)
    at mounted_app (/Users/chriscalo/Projects/viaticus/node_modules/express/lib/application.js:230:10)
    at Layer.handle [as handle_request] (/Users/chriscalo/Projects/viaticus/node_modules/express/lib/router/layer.js:95:5)
    at trim_prefix (/Users/chriscalo/Projects/viaticus/node_modules/express/lib/router/index.js:317:13)
    at /Users/chriscalo/Projects/viaticus/node_modules/express/lib/router/index.js:284:7
    at Function.process_params (/Users/chriscalo/Projects/viaticus/node_modules/express/lib/router/index.js:335:12)
    at next (/Users/chriscalo/Projects/viaticus/node_modules/express/lib/router/index.js:275:10)
    at Immediate.<anonymous> (/Users/chriscalo/Projects/viaticus/node_modules/express/lib/application.js:233:9)

It seems that Goldpage is calling next() even when it sends a response, so later handlers also try to respond. If I insert the following logic, it fixes things:

import express from "express";
import goldpage from "goldpage";

const server = express();
export default server;

server.use(goldpage.express);

// prevent later handlers from being called if goldpage has responded
server.use((req, res, next) => {
  if (!res.headersSent) next();
});

I think the fix is to not call next() when Goldpage sends a response. (Is @universal-adapter/express calling next() even when a response has been sent?)

chriscalo commented 4 years ago

(By the way, I really appreciate that Goldpage calls next() at all so that later handlers are able to run. Some express integrations assume that no other handlers need to run after their code, which is really frustrating. So please keep calling next() when Goldpage can't respond to the current request. 🙏)

brillout commented 4 years ago

Fixed & new Goldpage version 0.5.4 published.

chriscalo commented 4 years ago

Working great, thanks!