Conversation
|
|
||
| # Unresolved Questions | ||
|
|
||
| - `flush` and `close` are asynchronous functions, and in the presence of multiple handlers we might need to await them. Should we call them without await? I wouldn't want to delay rendering. How do we catch errors if they fail? |
There was a problem hiding this comment.
I imagine an adapter could provide something like waitUntil which doesn't block the request, otherwise block the request
There was a problem hiding this comment.
Possibly, but probably we can forget of the flush function. We will revisit once waitUntil lands in the next minor
Co-authored-by: Florian Lefebvre <contact@florian-lefebvre.dev>
| }); | ||
| ``` | ||
|
|
||
| ### `logHandlers.node(config?)` |
There was a problem hiding this comment.
From the user perspective, is there any real difference between the console and node loggers? They do the same, right, one just only works in Node, in which case, why have it?
There was a problem hiding this comment.
They don't do the same thing.
Node.js uses stdout/stderr to print the messages.
Console prints to console via info/warn/error
There was a problem hiding this comment.
Console always prints to stdout/stderr though. Do you know if there's an advantage to having both? Or did you do it that way because we already had both in the codebase (which IIRC is just historical and not purposefully designed).
There was a problem hiding this comment.
Console always prints to stdout/stderr though
Do you mean that the runtime (Node.js, workderd, etc.) does that? If so, then it's an implementation detail.
Or did you do it that way because we already had both in the codebase (which IIRC is just historical and not purposefully designed).
Yes we do. CLI and Pipeline use two different loggers. CLI uses a node.js logger, while the pipeline uses the console logger. Additionally, I didn't exclude the idea that a user could create a client logger, so I wanted to keep things open ended for now.
|
Call for consensus is now open. |
|
👍🏻 looks good |
|
👍 sounds good to me |
Co-authored-by: Florian Lefebvre <contact@florian-lefebvre.dev>
|
Just used a user query to dogfood this API — really handy way for people to customise stuff! Here’s the custom logger I made in case it’s of interest: https://stackblitz.com/edit/github-cf392aa7?file=astro.config.mjs,custom-logger.js Some quick feedback/ideas based on trying it:
Take all that with a grain of salt as just the experience for a specific kind of logger. But thought it would be helpful to share. |
|
We decided against having For the options type safety, that's good feedback. I think the RFC and the docs on custom loggers should be updated to match the official pattern: function pinoLogger(config: Options): LoggerHandlerConfig {
return {
entrypoint: '...',
config
}
}We have a similar situation for session drivers. You don't have to use this pattern but we only document this one because it's better DX |
|
Oh, and just remembered another thing that tripped me up getting started: The docs show adding a custom logger from an npm package entry point entrypoint: "@org/custom-logger",But most often people will be adding a local file to their project when building a custom logger, I’d guess. Or at least would do so initially before bundling it up to be published. It took me quite a bit of trial and error to find a way to specify a local file that worked in all contexts, ending up with: entrypoint: fileURLToPath(new URL('./custom-logger.js', import.meta.url)),That is definitely not super friendly — for most our APIs, e.g. like |
Summary
Links