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

A way to validate things on server initialization #6147

Closed
Zamiell opened this issue Aug 21, 2022 · 6 comments · Fixed by #6179
Closed

A way to validate things on server initialization #6147

Zamiell opened this issue Aug 21, 2022 · 6 comments · Fixed by #6179
Assignees

Comments

@Zamiell
Copy link

Zamiell commented Aug 21, 2022

Describe the problem

Related to #1538.

In other server solutions, a common pattern is check for the presence of necessary environment variables on startup, for the purposes of showing a friendly error message.

For example:

const { DATABASE_URI } = process.env;

if (DATABASE_URI === undefined || DATABASE_URI === "") {
  console.error(
    'The "DATABASE_URI" environment variable is not set. This is necessary in order for the server to function.',
  );
  console.error(
    'If you are deploying to Node, did you forget to copy the ".env.example" file to ".env" and fill in the values?',
  );
  console.error(
    "If you are deploying to Vercel, did you forget to add this environment variable to your project settings page?",
  );
  process.exit(1);
}

Doing this kind of thing prevents end-users from having to troubleshoot more-complicated run-time errors. ("Why is the database logic failing? Is the app that I just downloaded bugged?")

In #1538, Rich-Harris recommends using "hooks.js", but this doesn't solve the problem, as the logic in that file will only be executed when a user actually surfs to the page, rather than on server initialization.

Describe the proposed solution

SvelteKit should expose an idiomatic way to perform server-start validation of this manner.

This would also an appropriate place to e.g. initiate necessary database connections, so that the app can e.g. fail early if the database password is wrong (rather than when the first user actually tries to use your product).

Importance

nice to have

@Conduitry
Copy link
Member

@Zamiell
Copy link
Author

Zamiell commented Aug 21, 2022

That's helpful, but what about users who are using other adapters, such as adapter-vercel?

In my example in the OP, I included this text:

console.error(
  "If you are deploying to Vercel, did you forget to add this environment variable to your project settings page?",
);

The idea here is to make it so that this friendly error message would appear in the Vercel "Deployment Status" box, pictured here:

image

In general, I think there is a use-case for providing an adapter-agnostic error message, so that the end-user gets valuable information regardless of the deployment platform that they are using.

@elliott-with-the-longest-name-on-github
Copy link
Contributor

I've argued several times for a "startup" hook that's guaranteed to run right when you start your built app. I think there are many reasons to provide one, this being one of them, but it's also not super straightforward when it should run, especially in environments with the concept of instances, such as serverless platforms.

@Rich-Harris
Copy link
Member

The only way to surface runtime errors is by actually running the app, and there's no environment-agnostic way to do that as part of a build step (e.g. in the Vercel case, we'd need to access the deployment, but deployment doesn't happen until after the build step is complete).

We could surface startup errors immediately rather than waiting for the first request, just by moving this logic...

if (!this.options.hooks) {
const module = await import(${s(hooks)});
this.options.hooks = {
handle: module.handle || (({ event, resolve }) => resolve(event)),
handleError: module.handleError || (({ error }) => console.error(error.stack)),
externalFetch: module.externalFetch || fetch
};
}
...into the server.init(...) method (which would become await server.init(...)):
init({ env }) {
const entries = Object.entries(env);
const prv = Object.fromEntries(entries.filter(([k]) => !k.startsWith('${
config.kit.env.publicPrefix
}')));
const pub = Object.fromEntries(entries.filter(([k]) => k.startsWith('${
config.kit.env.publicPrefix
}')));
set_private_env(prv);
set_public_env(pub);
this.options.public_env = pub;
}

I think we should do that; it would help in the adapter-node case, and provides all the benefits of a dedicated startup hook without the downsides. But it wouldn't be a general solution to this problem because I don't think there is a general solution.

@elliott-with-the-longest-name-on-github
Copy link
Contributor

I'm with Rich -- simply having hooks.js initialized on startup is plenty good. It allows for optimistic connections to databases, running startup validation, etc. without increasing the API footprint.

@siddhsql
Copy link

I created a src/hooks.js and put following in it:

const CLIENT_ID = import.meta.env.VITE_GOOGLE_CLIENT_ID;
if (!CLIENT_ID) {
    throw Error("FATAL: CLIENT_ID is not set");
}

but when I run the application using npm rundev it still runs when CLIENT_ID is not set. am i doing anything wrong?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants