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

Recipe for email notifications? #2025

Open
binarykitchen opened this issue Aug 9, 2024 · 1 comment
Open

Recipe for email notifications? #2025

binarykitchen opened this issue Aug 9, 2024 · 1 comment

Comments

@binarykitchen
Copy link

I'm digging this library and learning more about it. Grazie for all the hard work

Surprised there is no email support yet in the Pino Ecosystem:
https://github.com/pinojs/pino/blob/main/docs/ecosystem.md

So, raising two issues here:

  1. Lack of support for email reports (yes, I could do a PR and I am trying to fix, see next issue)
  2. Better TypeScript compatibility would be good. Yes, I've read https://getpino.io/#/docs/transports?id=typescript-compatibility, yet, encourage altering the API for better target support.

I'll explain ...

I've already searched in many places and haven't found a transporter yet for sending out emails at an error level. So I already tried implementing my own like:

  transportTargets.push({
    target: "./getLoggerEmailTransport.ts",
    level: LogLevel.ERROR,
  });

and

import build from "pino-abstract-transport";
import sendServerError from "../email/sendServerError";

function getLoggerEmailTransport() {
  const transport = build(async function (source) {
    for await (const obj of source) {
      try {
        sendServerError(obj);
      } catch (err) {
        console.log("Failed to report error", err);
      }
    }
  });

  return transport;
}

export default getLoggerEmailTransport;

... which will use the nodemailer.

This fails under ts-node or jiti with the following error message:

import build from "pino-abstract-transport";
^^^^^^

SyntaxError: Cannot use import statement outside a module

That's because of this difficult code inside pino's transport-stream:

    if (toLoad.endsWith('.ts') || toLoad.endsWith('.cts')) {
      // TODO: add support for the TSM modules loader ( https://github.com/lukeed/tsm ).
      if (process[Symbol.for('ts-node.register.instance')]) {
        realRequire('ts-node/register')
      } else if (process.env && process.env.TS_NODE_DEV) {
        realRequire('ts-node-dev')
      }
      // TODO: Support ES imports once tsc, tap & ts-node provide better compatibility guarantees.
      fn = realRequire(decodeURIComponent(target))
    } else {
      fn = (await realImport(toLoad))
    }

Yeah, there is the TODO comment to improve this. It's a bit ugly. I wonder why the target has to be a string. Can't it be a file import right away, something like this?

import getLoggerEmailTransport from "./getLoggerEmailTransport"

  transportTargets.push({
    target: getLoggerEmailTransport, <---- this, reuse the import, no string
    level: LogLevel.ERROR,
  });

Enough questions :)

Thoughts? Happy to discuss further here.

@mcollina
Copy link
Member

This would be an excellent transport to add! Would you like to publish it?

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

No branches or pull requests

2 participants