Conversation
|
Co-authored-by: Puppo <2527849+Puppo@users.noreply.github.com>
…oLogger Co-authored-by: Puppo <2527849+Puppo@users.noreply.github.com>
Puppo
force-pushed
the
copilot/add-configuration-best-practices-rule
branch
from
338b904 to
32adc89
Compare
| const schema = { | ||
| type: "object", | ||
| required: ["PORT", "DATABASE_URL"], | ||
| properties: { | ||
| NODE_ENV: { | ||
| type: "string", | ||
| default: "development", | ||
| }, | ||
| PORT: { | ||
| type: "number", | ||
| default: 3000, | ||
| }, | ||
| DATABASE_URL: { | ||
| type: "string", | ||
| }, | ||
| LOG_LEVEL: { | ||
| type: "string", | ||
| default: "info", | ||
| }, | ||
| }, | ||
| }; | ||
|
|
||
| async function configPlugin(fastify: FastifyInstance) { | ||
| await fastify.register(fastifyEnv, { | ||
| confKey: "config", | ||
| schema, | ||
| dotenv: true, | ||
| }); | ||
| } |
There was a problem hiding this comment.
@copilot can we review this section and use zod?
There was a problem hiding this comment.
Rewrote the env config section to use Zod with z.object(), z.enum(), z.coerce, and z.infer<typeof envSchema> for types. Replaced @fastify/env with a Zod-based config plugin. See commit e9d2758.
| interface FastifyInstance { | ||
| config: { | ||
| NODE_ENV: string; | ||
| PORT: number; | ||
| DATABASE_URL: string; | ||
| LOG_LEVEL: string; | ||
| }; | ||
| } |
There was a problem hiding this comment.
Now using z.infer<typeof envSchema> (exported as Env) for the module augmentation via declare module "fastify" { interface FastifyInstance { config: Env } }. See commit e9d2758.
| const envToLogger: Record<string, object> = { | ||
| development: { | ||
| level: "debug", | ||
| transport: { | ||
| target: "pino-pretty", | ||
| options: { | ||
| translateTime: "HH:MM:ss Z", | ||
| ignore: "pid,hostname", | ||
| }, | ||
| }, | ||
| }, | ||
| production: { | ||
| level: "info", | ||
| }, | ||
| test: { | ||
| level: "silent", | ||
| }, | ||
| }; | ||
|
|
||
| const environment = process.env.NODE_ENV ?? "development"; | ||
|
|
||
| const server = Fastify({ | ||
| logger: envToLogger[environment] ?? true, | ||
| }); |
There was a problem hiding this comment.
review this part and use the zod schema to handle the env instead of using the process.env object
There was a problem hiding this comment.
Updated the logger section to parse env with envSchema.parse(process.env) first, then use config.NODE_ENV and config.LOG_LEVEL instead of raw process.env. See commit e9d2758.
| const server = Fastify({ | ||
| // Keep prototype poisoning protection at 'error' (default) | ||
| onProtoPoisoning: "error", | ||
| onConstructorPoisoning: "error", | ||
|
|
||
| // Set a request timeout to protect against slow requests (DoS) | ||
| requestTimeout: 120_000, // 2 minutes | ||
|
|
||
| // Limit payload size to prevent abuse | ||
| bodyLimit: 1_048_576, // 1 MiB (default), adjust as needed | ||
|
|
||
| // Return 503 when server is closing for graceful shutdown | ||
| return503OnClosing: true, | ||
|
|
||
| // Close idle connections on shutdown for clean exits | ||
| forceCloseConnections: "idle", | ||
| }); |
There was a problem hiding this comment.
use a function to build the server, and accept options as parameter
There was a problem hiding this comment.
Wrapped security options in a buildServer function that accepts { config }: BuildServerOptions where config: Env comes from the Zod schema. See commit e9d2758.
|
|
||
| > For more control, use a specific IP, CIDR range, or count instead of `true`: | ||
| > `trustProxy: '127.0.0.1'` or `trustProxy: 1`. | ||
|
|
There was a problem hiding this comment.
Removed the pluginTimeout section entirely. See commit e9d2758.
| const server = Fastify({ | ||
| // Increase plugin loading timeout in development | ||
| // when dealing with slow database connections, etc. | ||
| pluginTimeout: process.env.NODE_ENV === "development" ? 60_000 : 10_000, | ||
| }); |
There was a problem hiding this comment.
use the env schema and create the build function
There was a problem hiding this comment.
Updated to use envSchema.parse(process.env) and pass the parsed config to buildServer({ config }), then use config.PORT and config.HOST for listen options. See commit e9d2758.
| port: Number(process.env.PORT) || 3000, | ||
| // Use 0.0.0.0 in Docker/containers, localhost for local dev | ||
| host: process.env.HOST || "0.0.0.0", |
There was a problem hiding this comment.
don't use the process.env but use the data from the envSchema
There was a problem hiding this comment.
Replaced all process.env reads with values from the parsed Zod config (config.PORT, config.HOST). See commit e9d2758.
|
|
||
| const __dirname = fileURLToPath(new URL(".", import.meta.url)); | ||
|
|
||
| const envToLogger: Record<string, object> = { |
There was a problem hiding this comment.
create a typescript type that match the config
There was a problem hiding this comment.
Added export type Env = z.infer<typeof envSchema> in src/schema/env.ts and used it in BuildServerOptions and the module augmentation. See commit e9d2758.
| const environment = process.env.NODE_ENV ?? "development"; | ||
|
|
||
| const server = Fastify({ | ||
| logger: options.logger ?? envToLogger[environment] ?? true, | ||
| trustProxy: options.trustProxy ?? false, | ||
| requestTimeout: 120_000, | ||
| bodyLimit: 1_048_576, | ||
| return503OnClosing: true, | ||
| forceCloseConnections: "idle", | ||
| pluginTimeout: environment === "development" ? 60_000 : 10_000, | ||
| }); | ||
|
|
There was a problem hiding this comment.
don't use the process.env but the envSchema please
There was a problem hiding this comment.
The complete buildServer factory now receives config: Env from the parsed Zod schema and uses config.NODE_ENV for logger selection. No more process.env in server.ts. See commit e9d2758.
| ### Handle Graceful Shutdown | ||
|
|
||
| **Correct (listen for termination signals and close cleanly):** | ||
|
|
||
| `src/app.ts` | ||
|
|
||
| ```ts | ||
| import buildServer from "./server.js"; | ||
|
|
||
| const server = buildServer(); | ||
|
|
||
| const signals: NodeJS.Signals[] = ["SIGINT", "SIGTERM"]; | ||
|
|
||
| for (const signal of signals) { | ||
| process.on(signal, async () => { | ||
| server.log.info(`Received ${signal}, shutting down gracefully`); | ||
| await server.close(); | ||
| process.exit(0); | ||
| }); | ||
| } | ||
|
|
||
| await server.listen({ | ||
| port: Number(process.env.PORT) || 3000, | ||
| host: process.env.HOST || "0.0.0.0", | ||
| }); | ||
| ``` |
There was a problem hiding this comment.
please use the close-with-grace package to handle graceful shutdown,
don't use the process.env but the envSchema.
show also how to handle the close in a plugin please
There was a problem hiding this comment.
Replaced the manual signal handling with close-with-grace, using config.PORT/config.HOST from the Zod schema. Also added a src/plugins/db.ts example showing plugin cleanup with the onClose hook when server.close() is called. See commit e9d2758.
…close-with-grace, remove pluginTimeout section Co-authored-by: Puppo <2527849+Puppo@users.noreply.github.com>
New rule covering Fastify server configuration best practices, sourced from the official Fastify documentation.
New rule:
rules/configuration.mdCovers the following topics with incorrect/correct examples:
z.object,z.enum,z.coerce) andz.inferfor TypeScript typesonProtoPoisoning,requestTimeout,bodyLimit,return503OnClosing,forceCloseConnectionsinside abuildServerfactorytrustProxy— Extracting real client IP/host/protocol behind reverse proxieslistenbinding — Using parsed env config for host/port (0.0.0.0 for containers, localhost for local dev)buildServerfactory — All options composed together, acceptingconfig: Envfrom the Zod schemaclose-with-grace— Automatic signal/error handling + plugin cleanup withonClosehookUpdated docs
SKILL.md— Added Configuration to rule table, usage section, and skill descriptionREADME.md— Added Configuration to available skills tableOriginal prompt
✨ Let Copilot coding agent set things up for you — coding agent works faster and does higher quality work when set up for your repo.