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

Log level functionality broken when describing transmit #1861

Closed
kyranjamie opened this issue Dec 7, 2023 · 18 comments
Closed

Log level functionality broken when describing transmit #1861

kyranjamie opened this issue Dec 7, 2023 · 18 comments
Labels

Comments

@kyranjamie
Copy link

kyranjamie commented Dec 7, 2023

We're using Pino in the Leather web extension project, with the config:

const pinoLogger = pino({
  enabled: !IS_TEST_ENV,
  level: 'info',
  browser: {
    asObject: false,
    transmit: {
      level: 'info',
      send(_level, logEvent) {
        if (!chrome) return;
        logs$.next(logEvent);
      },
    },
  },
});

Not sure when this was introduced, but lately, pinoLogger.debug statements have been logging to console, despite the level being info. In debugging this, I've noticed that when removing the browser.transmit object, the logging behaviour works as expected.

@jsumners
Copy link
Member

jsumners commented Dec 7, 2023

Please provide a minimal reproducible example. Doing so will help us diagnose your issue. It should be the bare minimum code needed to trigger the issue, and easily runnable without any changes or extra code.

You may use a GitHub repository to host the code if it is too much to fit in a code block (or two).

@adragich
Copy link
Contributor

Hi, I have the same bug in my app.

There is how to reproduce: https://playcode.io/1935966

@mcollina
Copy link
Member

Thanks for reporting! Would you like to send a Pull Request to address this issue? Remember to add unit tests.

@mcollina mcollina added the bug label Jul 12, 2024
@adragich
Copy link
Contributor

Do you run browser-test in CI? Locally it complains about outdated airtap strategy:

yarn browser-test
$ airtap --local 8080 test/browser*test.js
The --local flag has been removed in favor of providers.

Providers must be installed separately. You might like:

- airtap-manual      (https://github.com/airtap/manual)
- airtap-playwright  (https://github.com/airtap/playwright);
- airtap-system      (https://github.com/airtap/system).
error Command failed with exit code 1.

@adragich
Copy link
Contributor

Also having a problem with a commit.

Do you have any guideline for contributing except for https://github.com/pinojs/pino/blob/main/CONTRIBUTING.md? Environment requirements, how to test locally etc. I get error in precommit hook (tried with a dummy change in README file):

​ FAIL ​ test/pkg/pkg.test.js
 ✖ Command failed: ./test/pkg/index-node14

  test: worker test when packaged into executable using pkg
  at:
    line: 984
    column: 15
    file: node:internal/errors
    function: genericNodeError
  code: 1
  killed: false
  signal: null
  cmd: ./test/pkg/index-node14
  stdout: >
    TAP version 13

    # Subtest: pino.transport with worker destination overridden by bundler and mjs transport
        not ok 1 - Invalid host defined options
          ---
          stack: >
            eval (eval at <anonymous>
            (/snapshot/pino/node_modules/real-require/src/index.js:4:20), <anonymous>:3:1)

            start (/snapshot/pino/node_modules/thread-stream/lib/worker.js:33:23)

            Object.<anonymous> (/snapshot/pino/node_modules/thread-stream/lib/worker.js:96:1)

            Module._compile (pkg/prelude/bootstrap.js:1930:22)
          at:
            line: 3
            column: 1
            file: <anonymous>
            evalOrigin: <anonymous>
            evalLine: 4
            evalColumn: 20
            evalFile: /snapshot/pino/node_modules/real-require/src/index.js
            function: eval
          type: TypeError
          tapCaught: uncaughtException
          test: pino.transport with worker destination overridden by bundler and mjs transport
          ...

        1..1
        # failed 1 test
    not ok 1 - pino.transport with worker destination overridden by bundler and mjs transport # time=41.037ms


    not ok 2 - /var/folders/g9/l9dygt1j4vl55q_7855542k00000gn/T/pino-34896-82a26307ffffc0abb35f210a hasn't been created within 10000 ms. File not yet created.
      ---
      stack: |
        Timeout.<anonymous> (/snapshot/pino/test/helper.js:0)
      tapCaught: returnedPromiseRejection
      test: pino.transport with worker destination overridden by bundler and mjs transport
      ...

    Error: ENOENT: no such file or directory, unlink '/var/folders/g9/l9dygt1j4vl55q_7855542k00000gn/T/pino-34896-82a26307ffffc0abb35f210a'
        at unlinkSync (fs.js:1256:3)
        at process.<anonymous> (/snapshot/pino/test/helper.js:0)
        at process.emit (events.js:412:35)
        at process.<anonymous> (/snapshot/pino/node_modules/source-map-support/source-map-support.js:516:21)
        at processEmit [as emit] (/snapshot/pino/node_modules/signal-exit/index.js:199:34) {
      errno: -2,
      syscall: 'unlink',
      code: 'ENOENT',
      path: '/var/folders/g9/l9dygt1j4vl55q_7855542k00000gn/T/pino-34896-82a26307ffffc0abb35f210a'
    }

    # unlink files

    # unliking /var/folders/g9/l9dygt1j4vl55q_7855542k00000gn/T/pino-34896-82a26307ffffc0abb35f210a

    # unlink completed

    1..2

    # failed 2 of 2 tests

    # time=10332.552ms
  stderr: ""
  tapCaught: returnedPromiseRejection

Node v20.12.1
yarn 1.22.17

@jsumners
Copy link
Member

You should be able to:

  1. Clone the repository
  2. Navigate into the repository directory
  3. npm install && npm run test

That should be all that is necessary for development of Pino.

@mcollina
Copy link
Member

Do you run browser-test in CI? Locally it complains about outdated airtap strategy:

Just run npm test. it seems airtap support is 100% broken and will require some updates. A PR for that would be nice, but it's out of scope.

@mcollina
Copy link
Member

Ref #2004

@adragich
Copy link
Contributor

Reinstalled packages with npm, tests seem to run now. May I have a permission to push feature branch?

@jsumners
Copy link
Member

Reinstalled packages with npm, tests seem to run now. May I have a permission to push feature branch?

Please review https://jrfom.com/posts/2017/03/08/a-primer-on-contributing-to-projects-with-git/

@adragich
Copy link
Contributor

I have tried to push and receive access error:

remote: Permission to pinojs/pino.git denied to adragich.
fatal: unable to access 'https://github.com/pinojs/pino.git/': The requested URL returned error: 403

what exactly am I missing?

@jsumners
Copy link
Member

jsumners commented Jul 12, 2024

https://jrfom.com/posts/2017/03/08/a-primer-on-contributing-to-projects-with-git/#fork-the-project

For remainder of this article we will assume that you want to collaborate on the awesome Pino project. Our first step is to create a copy of the project in our account on GitHub. So we navigate to [https://github.com/pinojs/pino] in a web browser and click “Fork” button

@adragich
Copy link
Contributor

Cool. Sorry, skipped the fork section. Would be nice to link this guide in the CONTRIBUTING.MD

@jsumners
Copy link
Member

Cool. Sorry, skipped the fork section. Would be nice to link this guide in the CONTRIBUTING.MD

Please submit PRs with improvements.

@adragich
Copy link
Contributor

adragich commented Jul 12, 2024

Thanks for review, waiting for the release :)

The latest PR is out of the scope of the bug

@adragich
Copy link
Contributor

Do you have a release date planned? The issue is closed but there is no version with the fix

@jsumners
Copy link
Member

Releases happen when one of the maintainers has time to work on it.

https://github.com/pinojs/pino/releases/tag/v9.3.0

Copy link

This issue has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Aug 16, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

No branches or pull requests

4 participants