getsentry / sentry-javascript

Official Sentry SDKs for JavaScript
https://sentry.io
MIT License
7.85k stars 1.55k forks source link

captureConsoleIntegration doesn't work on deno #12400

Open brc-dd opened 3 months ago

brc-dd commented 3 months ago

Is there an existing issue for this?

How do you use Sentry?

Sentry Saas (sentry.io)

Which SDK are you using?

@sentry/deno

SDK Version

8.7.0

Framework Version

1.44.1

Link to Sentry event

No response

SDK Setup

// foo.ts

import * as Sentry from 'npm:@sentry/deno'

Sentry.init({
  dsn: '...',
  integrations: [Sentry.captureConsoleIntegration()],
})

setTimeout(() => {
  console.error(new Error('This is an error'))
  prompt('Press Enter to exit...')
}, 100)

Steps to Reproduce

  1. run deno run -A foo.ts

Expected Result

It sends the error to Sentry.

Actual Result

Nothing happens.

If I manually do something like this it works, but the integration isn't working:

const orig = console.error
console.error = (...args: unknown[]) => {
  if (args.length === 1 && args[0] instanceof Error) {
    Sentry.captureException(args[0])
  }
  orig(...args)
}
brc-dd commented 3 months ago

It was working fine with 7.116.0

See comments below.

mydea commented 3 months ago

hey, thanks for writing in!

If you add debug: true in your init() call, what logs do you see?

brc-dd commented 3 months ago

Shows installed there:

Sentry Logger [log]: Integration installed: InboundFilters
Sentry Logger [log]: Integration installed: FunctionToString
Sentry Logger [log]: Integration installed: LinkedErrors
Sentry Logger [log]: Integration installed: Dedupe
Sentry Logger [log]: Integration installed: Breadcrumbs
Sentry Logger [log]: Integration installed: DenoContext
Sentry Logger [log]: Integration installed: ContextLines
Sentry Logger [log]: Integration installed: NormalizePaths
Sentry Logger [log]: Integration installed: GlobalHandlers
Sentry Logger [log]: Integration installed: CaptureConsole
Error: This is an error
    at file:///Users/brc-dd/foo/foo.ts:10:17
    at eventLoopTick (ext:core/01_core.js:203:13)
Press Enter to exit... 
mydea commented 3 months ago

Ah, I see, I can reproduce this πŸ€” It seems that console instrumentation is not working at all, also there are no breadcrumbs anymore πŸ€” It does seem to work for me when importing from denoland, though:

import * as Sentry from "https://deno.land/x/sentry/index.mjs";

vs this does not work:

import * as Sentry from "npm:@sentry/deno";

cc @timfish or @AbhiPrasad do you have any clue what could cause this, possibly?

timfish commented 3 months ago

Unfortunately Deno has two global scopes! There's one for regular Deno packages and one for npm scoped modules.

This means that when you init Sentry via the npm package, it does not instrument the Deno globals. If you init Sentry with the Deno land package, the npm globals are not instrumented.

Let me find the reference issues...

timfish commented 3 months ago

Okay so I remembered it wrong, there aren't multiple globals but Deno returns different properties depending on the package scope and this affects console:

https://github.com/denoland/deno/issues/20826

brc-dd commented 3 months ago

It does seem to work for me when importing from denoland

Ah for v7, I was using denoland. Hadn't tested v8 from there. Yeah from deno.land/x/ it seems to work.

there aren't multiple globals but Deno returns different properties depending on the package scope

Makes sense I guess. So a fix would be to patch both polyfilled console and deno's console? πŸ‘€

timfish commented 3 months ago

This might work but I haven't tested it:

import * as Sentry from "https://deno.land/x/sentry/index.mjs";
import { captureConsoleIntegration } from "npm:@sentry/deno";

const npmCaptureConsole = captureConsoleIntegration();
// Rename so de-duplication doesn't remove one of the instances
npmCaptureConsole.name += '-npm';

Sentry.init({
   dsn: '__DSN__',
   integrations: [npmCaptureConsole]
})
brc-dd commented 3 months ago

Ah, no doesn't work for errors logged from npm packages. MRE:

import * as Sentry from 'https://deno.land/x/sentry@8.8.0/index.mjs'
import { captureConsoleIntegration } from 'npm:@sentry/deno'

const npmCaptureConsole = captureConsoleIntegration()
npmCaptureConsole.name += '-npm'

Sentry.init({
  dsn: '__DSN__',
  integrations: [
    Sentry.captureConsoleIntegration(),
    npmCaptureConsole,
  ],
  debug: true,
})

await new Promise((resolve) => setTimeout(resolve, 1000))

import { log } from 'npm:@brc-dd/console-error@0.0.2'

log() // <-- this isn't reported

console.error('error from deno') // <-- only this gets reported

await Sentry.flush()

And if you swap those:

import { captureConsoleIntegration } from 'https://deno.land/x/sentry@8.8.0/index.mjs'
import * as Sentry from 'npm:@sentry/deno'

const denoCaptureConsole = captureConsoleIntegration()
denoCaptureConsole.name += '-deno'

Sentry.init({
  dsn: 'https://4d9c3fc45bb25d00ae8b8818dede1297@o281429.ingest.us.sentry.io/4507389454319618',
  integrations: [
    Sentry.captureConsoleIntegration(),
    denoCaptureConsole,
  ],
  debug: true,
})

await new Promise((resolve) => setTimeout(resolve, 1000))

import { log } from 'npm:@brc-dd/console-error@0.0.2'

log() // <-- this gets reported now

console.error('error from deno') // <-- this is no longer reported

await Sentry.flush()

I'm guessing there is some patching going on in Sentry.init too, or maybe those integrations rely on something set by Sentry.init on globalThis?