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

Feature request: Ability to build for zeit/now serverless functions #16

Open
barbalex opened this issue Sep 7, 2019 · 15 comments
Open
Labels
enhancement New feature or request help wanted Extra attention is needed

Comments

@barbalex
Copy link

barbalex commented Sep 7, 2019

I guess only the deploy script would have to be modified?

But I am a complete backend noob so just asking.

@benjie
Copy link
Member

benjie commented Sep 9, 2019

@enisdenjo mentioned on Discord that there's a bug in the Zeit pkg bundler which causes Maximum call stack size exceeded errors periodically. Reference: vercel/pkg#681

Not sure if that's relevant or not.

I am a serverless newbie myself so I'm not sure how hard it would be to support Now.

@benjie benjie added enhancement New feature or request help wanted Extra attention is needed labels Sep 9, 2019
@enisdenjo
Copy link

Since lambda functions do just one task and then die, the related pkg call stack issue might not prevail here.

Through my personal experience the server was occasionally entering an invalid state which caused lots of Maximum call stack size exceeded issues on some queries (not all were affected, some were working some not). But this invalid state gets reached only when the server is running after a while; given that PostGraphile won't be running long in a lamda function, my guess is it will not occur as often... But still, haven't tested it, so don't take my word for it. :)

Maybe use a different node compiler? nexe?

@enisdenjo
Copy link

enisdenjo commented Sep 9, 2019

AWS lambda has a Node.js v10 runtime. So you don't even need to use any node compilation technologies.

@benjie
Copy link
Member

benjie commented Sep 9, 2019

Is Now lambda-compatible or does it have it's own way of running? We use the aws-serverless-express stub (it doesn't and we don't use express itself, I think the name of this package is much like the "Java" in "JavaScript" - a play on popularity) to expose an HTTP middleware as a serverless function.

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

@konsumer
Copy link

konsumer commented May 10, 2020

It's lambda-compatible, but they run node apps through pkg to bundle your whole app into a single file, for smaller (tree-shaken) file-sizes and ES7 syntax, among other things. I think it also simplifies & speeds up the compilation step, as it uses a fast AST-aware cache.

It's totally possible to build the graphile cache in the "build phase" and then import the cache, in the code:

const { createPostGraphileSchema } = require('postgraphile')
const { Pool } = require('pg')

const schemas = process.env.DATABASE_SCHEMAS
  ? process.env.DATABASE_SCHEMAS.split(',')
  : ['app_public']

async function main () {
  console.log('Generating cache')
  const pgPool = new Pool({
    connectionString: process.env.DATABASE_URL
  })
  await createPostGraphileSchema(pgPool, schemas, {
    writeCache: `${__dirname}/../src/postgraphile_cache.json`
  })
  await pgPool.end()
}

main().then(null, e => {
  console.error(e)
  process.exit(1)
})

then use it like this:

import { postgraphile } from 'postgraphile'

// this works, but how do I actually use the cache?
import cache from '../src/postgraphile_cache.json'

// yours go here...
const options = {}

const handler = postgraphile(process.env.DATABASE_URL, schemas, {
  ...options,
  // not this, but what should I do here?
  // readCache: `${__dirname}/../src/postgraphile_cache.json`
})

The problem here is that readCache needs a filename, and I can't seem to find an option to pass a pre-made cache (in this case, named cache) directly to postgraphile.

__dirname is overwritten with /, unhelpfully, but also the deploy setup only pushes what it has in the bundle, so a JSON file wouldn't be included, automatically.

I think there are some options in now for including asset sort of files (non-imported) in the bundle that gets run on lambda (and pkg does interesting things to include compiled native node-libs, similar to this.) There may also be a hacky way to do it, by putting the JSON file in the public static files, then making a web-request to grab it, but I hate that. It would be ideal if I could just import the JSON directly (it would be inside the bundle) as this would simplify the end-users configuration, and work more reliably.

Does anyone know a way to inject the cache directly into postgraphile?

@benjie
Copy link
Member

benjie commented May 11, 2020

Looks like just requiring the JSON should work:

import { postgraphile } from 'postgraphile'

// this works, but how do I actually use the cache?
import cache from '../src/postgraphile_cache.json'

// yours go here...
const options = {}

const handler = postgraphile(process.env.DATABASE_URL, schemas, {
  ...options,
  readCache: require(`${__dirname}/../src/postgraphile_cache.json`)
})

(If you're in TypeScript you might need an (require(...) as any) to bypass types.)

@konsumer
Copy link

Looks like just requiring the JSON should work:

Hmm, no, that is a JSON object, not a filename (as readCache expects)

@enisdenjo
Copy link

enisdenjo commented May 11, 2020

readCache can parse objects since [email protected] (check out: graphile/graphile-engine#479). The documentation needs an update.

@konsumer
Copy link

That is awesome, thanks @benjie and @enisdenjo I'd be happy to make a demo that deploys to now.

If that's the case, then I think in the above example I should be able to just pass it cache directly:

import { postgraphile } from 'postgraphile'

import readCache from '../src/postgraphile_cache.json'

// yours go here...
const options = {readCache}

const handler = postgraphile(process.env.DATABASE_URL, schemas, options)

@konsumer
Copy link

konsumer commented May 11, 2020

Oh, wait, there already is one at bottom of readme. doh!

https://github.com/ggascoigne/now-postgraphile

But weirdly, it uses a file. I'm pretty sure that wouldn't work:
https://github.com/ggascoigne/now-postgraphile/blob/master/api/graphql.ts#L36

@benjie
Copy link
Member

benjie commented May 12, 2020

I assume it must work since there's a demo of it: https://now-postgraphile.now.sh/

@konsumer
Copy link

I assume it must work since there's a demo of it: https://now-postgraphile.now.sh/

I couldn't get it to work. Maybe I am doing something wrong. Here is what I did:

First, there is a catch-22 with secrets. You can't setup secrets without a project, and you can't link it to a project without a deploy (which in this case, doesn't work without secrets.) I removed the env lines, for an initial now deploy, then added them back, and then ran ./secrets.sh(after changing #!/usr/local/bin/bash to #!/usr/bin/bash.) This got the initial secrets/project setup.

To get stuff to run (I was getting path errors), I changed this in makeCache.js:

const pgPool = getPool('./shared/')

to this:

const pgPool = getPool(`${__dirname}/../`)

and set this in env-file:

DATABASE_SSL_CERT="shared/rds-combined-ca-bundle.pem"

Next, I made a script that looked like this, in shared/createSchema.js:

const { getSchemas, getPool } = require('./config')

const db = getPool()

const { options: { user } } = db
const [schemaName] = getSchemas()

const run = async () => {
  await db.query(`CREATE SCHEMA IF NOT EXISTS ${schemaName} AUTHORIZATION ${user};`)
  await db.query(`CREATE TABLE ${schemaName}.thing(
    id INT primary key,
    stritem TEXT,
    intitem INT
  );`)
}
run()

After this, I ran node shared/makeCache.js (which really should be in the now build step, so it creates it's own cache on deploy.) This seemed to work ok, as it made a cache file, and there were no errors.

Then I run now and got this:

https://now-postgraphile.dkonsumer.now.sh/

Note, there are no schemas, but it seems to run graphiql ok.

I checked console to see what was up with Error 502 on introspection query:

Request URL: https://now-postgraphile.dkonsumer.now.sh/api/graphql
Request Method: POST
Status Code: 502 
Remote Address: 52.38.79.87:443
Referrer Policy: no-referrer-when-downgrade
cache-control: s-maxage=0
content-type: text/plain; charset=utf-8
date: Tue, 12 May 2020 19:40:11 GMT
server: now
status: 502
strict-transport-security: max-age=63072000; includeSubDomains; preload
x-now-id: pdx1::9cvcd-1589312411727-71e552780467
x-now-trace: pdx1
x-robots-tag: noindex
x-vercel-id: pdx1::9cvcd-1589312411727-71e552780467
:authority: now-postgraphile.dkonsumer.now.sh
:method: POST
:path: /api/graphql
:scheme: https
accept: */*
accept-encoding: gzip, deflate, br
accept-language: en-US,en;q=0.9
content-length: 1728
content-type: application/json
dnt: 1
origin: https://now-postgraphile.dkonsumer.now.sh
referer: https://now-postgraphile.dkonsumer.now.sh/
sec-fetch-dest: empty
sec-fetch-mode: cors
sec-fetch-site: same-origin
user-agent: Mozilla/5.0 (X11; Linux x86_64) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/81.0.4044.138 Safari/537.36

An error occurred with this application.

NO_STATUS_CODE_FROM_FUNCTION

On now, the function-log has this:

[POST] /api/graphql
12:44:03:56
2020-05-12T19:44:03.719Z	3ceb03a9-6795-487b-91b0-02d88820dca7	ERROR	A serious error occurred when building the initial schema. Exiting because `retryOnInitFail` is not set. Error details:
Error: Expected cache to contain key: PgIntrospectionPlugin-introspectionResultsByKind-v4.6.0
    at persistentMemoizeWithKey (/var/task/node_modules/postgraphile-core/node8plus/index.js:138:27)
    at introspect (/var/task/node_modules/graphile-build-pg/node8plus/plugins/PgIntrospectionPlugin.js:273:57)
    at PgIntrospectionPlugin (/var/task/node_modules/graphile-build-pg/node8plus/plugins/PgIntrospectionPlugin.js:468:45)
    at Object.getBuilder (/var/task/node_modules/graphile-build/node8plus/index.js:167:11)
    at Object.exports.createPostGraphileSchema (/var/task/node_modules/postgraphile-core/node8plus/index.js:222:21)
    at createGqlSchema (/var/task/node_modules/postgraphile/src/postgraphile/postgraphile.ts:105:23)
RequestId: 3ceb03a9-6795-487b-91b0-02d88820dca7 Error: Runtime exited with error: exit status 34
Runtime.ExitError

If injecting the object works, directly, I can make a lighter example that is a bit simpler, and it should work fine. This code doesn't do that (it uses the file-location of the JSON cache) which didn't work for me in my tests, at least a month or so ago, or in my own test project, 3 days ago, when I first commented on this issue.

@konsumer
Copy link

konsumer commented May 13, 2020

I made this which works with undocumented readCache object, and has a bit simpler setup and clearer directions.

@benjie
Copy link
Member

benjie commented May 13, 2020

It could be that the cache was generated with a different version, or that the import cache from ... isn't behaving identically to how JSON.parse(fs.readFileSync(...)) would.

@konsumer
Copy link

konsumer commented May 13, 2020

It could be that the cache was generated with a different version,

I tried with pinned versions of postgraphile between my box and remote, and also has there been new published of postgraphile in the last 3 days? Would those minor changes effect the cache-writing portion? If so, then there would be a much bigger problem of breaking changes happening on minor version updates of postgraphile, but neither of those things are the case. Last release was 16 days ago (4.7.0), so both installs are in the same version-window, happening 5 minutes apart, 3 days ago, from fresh npm caches, on 2 linux boxes (my local and remote lambda builder.)

or that the import cache from ... isn't behaving identically to how JSON.parse(fs.readFileSync(...)) would.

I disagree. Importing vs JSON.parse(fs.readFileSync(...)) differences are truly negligible, on first load. After first load the main difference is that the object is in the require-cache, which dies after the run-cycle. This is pretty easy to verify with a console.log of the object/read & JSON.parsed file.

I made a test project to explore these claims, and see how now handles various ways of "load JS object from this JSON file":

https://github.com/konsumer/test-now-json

https://test-now-json.now.sh/api/require
https://test-now-json.now.sh/api/bad_import
https://test-now-json.now.sh/api/fs

All 3 API routes should do the same thing, and it illustrates a recent change for now. It's been a pain-point for lots people since now 2.0 that you can't dynamically read a JSON file. You'll notice "bad_import" is working, which is closest to what you proposed above. Until very recently, only the require api-route would have actually worked. Pkg doesn't actually include the files, from the real filesystem location that are imported, it bundles them and points to the inlined object (similar to what babel/requirejs/browserify/etc do) and until recently, it didn't include any files that were not directly required/imported, before runtime, unless the builder told it to (through options in now.json.)

Edit: I put the wrong URL in for bad_import, which works fine.

Since I wrote a now-graphile-demo that actually works out of the box with no fussing, I'm not really into following along anymore with that non-functioning code, as it's a bit less organized, and convoluted in parts, and mine seems to work great. I think there is probably some other minor little thing that needs to be fixed, but why would I bother when I have a better, actually working code-example, I wrote myself?

Due to the path changes I had to make to get it to run at all, I suspect the code that is deployed on the other demo isn't exactly what's in it's repo, which makes it even harder to troubleshoot.

My main point above was "a JS object is not the same as a string filename" and we have cleared up that there is an undocumented way to load an object directly, which is exactly what I was looking for. I tested & verified this by making a working project that uses it.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request help wanted Extra attention is needed
Projects
None yet
Development

No branches or pull requests

4 participants