Elderjs / elderjs

Elder.js is an opinionated static site generator and web framework for Svelte built with SEO in mind.
https://elderguide.com/tech/elderjs/
MIT License
2.12k stars 53 forks source link

Redirect from middleware hook #185

Closed bbuhler closed 3 years ago

bbuhler commented 3 years ago

Hello 👋️

I try to perform an HTTP redirect from a middleware hook based on route request object.

  {
    hook: 'middleware',
    name: 'redirect',
    description: 'Handle route redirects',
    priority: 60,
    run({ res, req, serverLookupObject, next })
    {
      const lookedupRequest = serverLookupObject[req.url];

      if (lookedupRequest?.redirectTo)
      {
        res.redirect(lookedupRequest.redirectTo);
        res.headerSent = true;
        // next();
      }

      return { res };
    }
  },

It somehow works, but I get 'Cannot set headers after they are sent to the client' errors in the logs.

How can I prevent that the subsequent hooks and rendering is being executed? Is there a smarter way to archive that?

nickreese commented 3 years ago

Just shipped a fix for this in v1.4.5.

Here is the hook that was working as expected (hook works with polka and I'd bet also with express):

  {
    hook: 'middleware',
    name: 'redirect',
    description: 'Handle route redirects',
    priority: 60,
    run({ res, req, next }) {
      if (req.path.includes('redirect')) {
        const str = `Redirecting to ${`http://example.com`}`;
        console.log(str);
        res.writeHead(302, {
          Location: `http://example.com`,
          'Content-Type': 'text/plain',
          'Content-Length': str.length,
        });
        res.end(str);
      }
    },
  },
bbuhler commented 3 years ago

Hi @nickreese, it is still throwing exceptions with version 1.4.6.

  Error [ERR_HTTP_HEADERS_SENT]: Cannot set headers after they are sent to the client
      at ServerResponse.setHeader (_http_outgoing.js:561:11)
      at handleRequest (/home/bbuhler/Documents/project-name/node_modules/@elderjs/elderjs/build/routes/prepareRouter.js:108:17)
      at processTicksAndRejections (internal/process/task_queues.js:95:5)
      at async /home/bbuhler/Documents/project-name/node_modules/@elderjs/elderjs/build/utils/prepareRunHook.js:46:44
      at async processHook (/home/bbuhler/Documents/project-name/node_modules/@elderjs/elderjs/build/utils/prepareRunHook.js:41:13)
      at async prepServer (/home/bbuhler/Documents/project-name/node_modules/@elderjs/elderjs/build/utils/prepareServer.js:8:9) {
    code: 'ERR_HTTP_HEADERS_SENT'
  }

I think res.headerSent was removed in favor of native res.headersSent in newer express versions.

nickreese commented 3 years ago

@bbuhler Looks like this was either a typo or I was referencing old express docs. Either way, it now checks for plural and singular.