ForestAdmin / forest-express

🧱 Dependency of Express Lianas for Forest Admin
GNU General Public License v3.0
72 stars 21 forks source link

[-] Initialization - Expose a way to ensure that the initialization is done after `liana.init` call #180

Closed atwoodjw closed 4 years ago

atwoodjw commented 6 years ago

Expected behavior

Init should return a promise or accept a callback, so we know when the returned app is ready to service requests.

Actual behavior

Init returns an app, which is not ready to service requests. Init has a fairly complex promise chain, which is not resolved until after the app is returned.

Failure Logs

Serverless: ANY /forest/User (λ: forest)
Serverless: The first request might take a few extra seconds
Serverless: [404] {"statusCode":404,"body":"<!DOCTYPE html>\n<html lang=\"en\">\n<head>\n<meta charset=\"utf-8\">\n<title>Error</title>\n</head>\n<body>\n<pre>Cannot GET /forest/User</pre>\n</body>\n</html>\n","headers":{"x-powered-by":"Express","vary":"Origin","content-security-policy":"default-src 'self'","x-content-type-options":"nosniff","content-type":"text/html; charset=utf-8","content-length":"150","date":"Tue, 05 Jun 2018 20:57:14 GMT","connection":"close"},"isBase64Encoded":false}

Context

Our app is built on AWS Lambda. To support Forest, we created a Lambda that wraps the forest-express-sequelize example.

Unlike a traditional Express app, the Express app in the Lambda is created and immediately receives a requests, before the promise chain in init is resolved. This results in a 404 from Express.

forest.js
import awsServerlessExpress from "aws-serverless-express";

import Server from "./server";

async function handler(event, context) {
  const server = await Server();

  return new Promise(resolve => {
    awsServerlessExpress.proxy(server, event, {
      ...context,
      succeed: process.env.IS_OFFLINE ? context.succeed : resolve
    });
  });
}

export { handler };
server.js
import awsServerlessExpress from "aws-serverless-express";
import express from "express";
import forestExpressSequelize from "forest-express-sequelize";

import environment from "./environment";

import Db from "../models";

let _server;
export default async function() {
  if (_server) return _server;

  const db = await Db();

  const config = forestExpressSequelize.init({
    authSecret: environment.forestAuthSecret,
    envSecret: await environment.forestEnvSecret(),
    sequelize: db.sequelize
  });

  const app = express();
  app.use(config);

  _server = awsServerlessExpress.createServer(app);

  return _server;
}
tlubz commented 6 years ago

We have been seeing the same issue in our implementation. Serverless/lambda really seems like the way to go for hosting these liana nanoservices. We are using forest-express-sequelize (through lumber-cli). Having a persistent server to proxy light admin traffic to a postgres database seems like overkill.

We have been forced to use a pretty clumsy workaround here, and wrap our AwsServerlessExpress.proxy(...) call in a setTimeout(..., 200) block.

It would be great if the ForestExpress.init call set a promise on the returned object, or returned a promise, as the app returned is essentially not usable until the readdirAsync promise chain settles.

Alternatively, exposing a "ready" callback in the opts could be useful (e.g. similar to https://github.com/ForestAdmin/forest-express/pull/181).

Ni0rd commented 5 years ago

Hi, I am also experiencing this. I fixed it by disabling the auto schema sync : FOREST_DISABLE_AUTO_SCHEMA_APPLY = true and also by implementing a very dirty hack to detect when the last request is made... here an exemple in TypeScript for Zeit Now.

import EventEmitter from 'events';
import forestExpressSequelize from 'forest-express-sequelize';
import forestExpressServiceIpWhitelist from 'forest-express/dist/services/ip-whitelist';
import { sequelize } from '../sequelize';

const app = forestExpressSequelize.init({
  envSecret: process.env.FOREST_ENV_SECRET,
  authSecret: process.env.FOREST_AUTH_SECRET,
  sequelize,
});

const emitter = new EventEmitter();

let ipWhitelistDone = false;

const old = forestExpressServiceIpWhitelist.retrieve;
forestExpressServiceIpWhitelist.retrieve = async (
  environmentSecret,
): Promise<void> => {
  await old(environmentSecret);
  ipWhitelistDone = true;
  emitter.emit('ipWhitelistReady');
};

function ipWhitelistReady(): Promise<void> {
  return new Promise((resolve): void => {
    if (ipWhitelistDone) {
      resolve();
      return;
    }
    emitter.on('ipWhitelistReady', resolve);
  });
}

app.use(async (req, res, next) => {
  await ipWhitelistReady();
  next();
});

export default app;
louisremi commented 5 years ago

At Mon Bel Appart, we've found a reliable way to detect when the Liana has successfully initialized by hijacking different functions of its API. Here's how we've done it: https://github.com/monbelappart/monbelappart/blob/master/src/utils/Forest/hijackLiana.ts

pangolingo commented 5 years ago

We're still seeing this issue since @atwoodjw reported it last year. As he said, the problem is that we’re running on AWS Lambda and the Forest library doesn’t have time to boot before it tries to handle the request. This causes intermittent 404 errors because sometimes a new Lambda is spun up and sometimes an existing Lambda is reused (when reusing a Lambda the library has been initialized and responds successfully).

We need a way to await the library initialization before asking it to handle the request.

slimee commented 4 years ago

Hi guys,

Happy to say you that (later is better than never) this will be handled in v6. Liana.init will return a promise that resolves after complete initialization.

p0wl commented 4 years ago

I want to use forest admin within aws lambda, but I cannot manage to add a production connection using the dashboard, I'm stuck in the "Set your environment variables" step. I tried debugging it, but I don't get any log output from forest or anything else. All I see is that the calss to /forest are answered with http 204 without any response.

I tried to use aws-serverless-express to wrap forest-admin in a lambda function, my entrypoint looks like this:

'use strict';

const awsServerlessExpress = require('aws-serverless-express');
const app = require('./app');
const server = awsServerlessExpress.createServer(app);

exports.handler = (event, context) => { awsServerlessExpress.proxy(server, event, context) };

and I have not changed app.js or anything else.

The serverless.yml is pretty simple as well:

service: myservice

provider:
  name: aws
  runtime: nodejs12.x
  stage: production
  region: eu-central-1

package:
  exclude:
    - '.env'

functions:
  forestadmin:
    handler: serverless.handler
    events:
      - httpApi: '*'
        cors: true
    environment:
      FOREST_ENV_SECRET: '...'
      FOREST_AUTH_SECRET: '...'
      FOREST_DISABLE_AUTO_SCHEMA_APPLY: true
      DATABASE_URL: 'postgres://...'
      DATABASE_SCHEMA: public
      DATABASE_SSL: false
      NODE_ENV: production
arnaudbesnier commented 4 years ago

Hi @p0wl, I am not sure to understand how this is related to the issue. If you need help to configure your aws lambda maybe you should ask for help here or create a dedicated issue.

arnaudbesnier commented 4 years ago

@atwoodjw @pangolingo @louisremi @davidbillamboz @tlubz thank you for your patience guys. This contribution is available in a new 6.0.0-beta.0 beta version.

As a new premajor version, it contains breaking changes, so please take the time to read the upgrade note and test all your changes in a development environment before moving it to upper stages (staging, production,...)

🌲🌲🌲