ovhemert / pino-applicationinsights

A transport for pino that sends messages to Azure Application Insights
MIT License
10 stars 12 forks source link

Convert `createWriteStream` to `createWriteStreamSync` #57

Closed devinrhode2 closed 1 year ago

devinrhode2 commented 2 years ago

Users will get an import error when updating. I think it should be pretty easy to fix.

This is a smaller diff of just the src changes, with no refactoring, no test updates, no doc updates, etc: https://github.com/ovhemert/pino-applicationinsights/pull/59 It will be easier first to understand the src changes in that PR

Checklist

Also adding a jsdoc typescript signature.

Closes #56

devinrhode2 commented 2 years ago

Running tests locally, I'm getting some confusing timeouts..

> pino-applicationinsights@2.1.0 test /Users/devinrhode2/repos/Atlas/pino-applicationinsights
> standard && tap test/*.test.js --coverage --100

 PASS  test/streams.test.js 4 OK 31.784ms
test/index.test.js 2> ApplicationInsights:An invalid instrumentation key was provided. There may be resulting telemetry loss [ 'blablabla' ]
test/applicationinsights.test.js 2> ApplicationInsights:An invalid instrumentation key was provided. There may be resulting telemetry loss [ 'blablabla' ]
 FAIL  test/applicationinsights.test.js
 ✖ timeout!

  test: TAP
  signal: SIGTERM
  handles:
    - type: TLSSocket
      events:
        - close
        - end
        - newListener
        - secure
        - session
        - free
        - timeout
        - agentRemove
        - error
  expired: TAP
  stack: |
    emit (node_modules/signal-exit/index.js:78:11)
    process.listener (node_modules/signal-exit/index.js:92:7)

 FAIL  test/index.test.js
 ✖ timeout!

  test: TAP
  signal: SIGTERM
  handles:
    - type: TLSSocket
      events:
        - close
        - end
        - newListener
        - secure
        - session
        - free
        - timeout
        - agentRemove
        - error
  expired: TAP
  stack: |
    emit (node_modules/signal-exit/index.js:78:11)
    process.listener (node_modules/signal-exit/index.js:92:7)

....

------------------------|---------|----------|---------|---------|-------------------
File                    | % Stmts | % Branch | % Funcs | % Lines | Uncovered Line #s 
------------------------|---------|----------|---------|---------|-------------------
All files               |   38.75 |    20.83 |    37.5 |   45.45 |                   
 applicationinsights.js |   16.95 |        5 |   16.67 |   21.74 | 9,18-76,83-87     
 index.js               |     100 |      100 |     100 |     100 |                   
 streams.js             |     100 |      100 |     100 |     100 |                   
------------------------|---------|----------|---------|---------|-------------------
devinrhode2 commented 2 years ago

Turns out, test output is unchanged relative to master, diff here: https://github.com/ovhemert/pino-applicationinsights/pull/57/commits/e283e7478a3d30782f2d11453f58cbdb28c899d1

stale[bot] commented 2 years ago

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

devinrhode2 commented 2 years ago

Well that's sad @ovhemert did you even setup/configure stale bot?

stale[bot] commented 2 years ago

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.