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

Support for Deno server #3009

Closed
8 tasks done
mhmo91 opened this issue Aug 31, 2020 · 38 comments · Fixed by #9206
Closed
8 tasks done

Support for Deno server #3009

mhmo91 opened this issue Aug 31, 2020 · 38 comments · Fixed by #9206
Assignees
Labels
Package: deno Issues related to the Sentry Deno SDK

Comments

@mhmo91
Copy link

mhmo91 commented Aug 31, 2020

Summary

SDK For https://deno.land/

A Deno SDK would require Sentry provides a package which follows the spec upon which Deno applications are built. Essentially it is based on a non-node JS runtime. To some extend the existing JS SDK can already run in such runtimes with recent changes post v7 (Vercel Edge for example)

    - Handling imports/dependencies (imports & lack of package.json)

Install

Product

Open Points:

  • Are there any specific around transaction naming conventions or similar which would be natively better to Deno, than Node for example?
    • Specifics around span operation naming conventions? (HTTP, DB, rpc, etc)
@BYK BYK transferred this issue from getsentry/sentry Oct 26, 2020
@gabeidx
Copy link

gabeidx commented Sep 2, 2021

Namespace is taken on the official registry: https://deno.land/x/sentry@deno

@AbhiPrasad
Copy link
Member

@smeubank Something we should consider as part of work in the future - building out an SDK for Deno.

@github-actions
Copy link
Contributor

This issue has gone three weeks without activity. In another week, I will close it.

But! If you comment or otherwise update it, I will reset the clock, and if you label it Status: Backlog or Status: In Progress, I will leave it alone ... forever!


"A weed is but an unloved flower." ― Ella Wheeler Wilcox 🥀

@gabeidx
Copy link

gabeidx commented Oct 21, 2021

@AbhiPrasad @smeubank would love to see an official Deno SDK… 🙏

@KaKi87
Copy link

KaKi87 commented Feb 21, 2022

NPM libraries that support browsers can usually be imported as ES modules thanks to JSPM.
So, the following code works in browser :

import { default as Sentry } from 'https://dev.jspm.io/npm:@sentry/browser@6.17.9';
Sentry.init({ dsn });
Sentry.captureException(new Error('Hello, World !'));

Since Deno is also compatible with libraries supporting browsers, this code should also work on Deno, but it doesn't :/
Ideas ?
Thanks

@AbhiPrasad
Copy link
Member

@KaKi87 It might be because the Node SDK has some default integrations that rely on node APIs - but that's just a guess of mine.

We rely on comments/emoji reactions for GH issues like these to determine what to work on, so please continue to let us know that this is important to you all!

@KaKi87
Copy link

KaKi87 commented Feb 24, 2022

the Node SDK has some default integrations that rely on node APIs

Yes, but I'm importing @sentry/browser, not @sentry/node 🤔

We rely on comments/emoji reactions for GH issues like these to determine what to work on, so please continue to let us know that this is important to you all!

Done 👍

@dgpgdev
Copy link

dgpgdev commented May 9, 2022

i'm looking forward this sdk release

@timfish
Copy link
Collaborator

timfish commented Sep 13, 2022

I've been actively looking at what changes are required to support other runtimes like Deno with the existing codebase!

Until all those changes are complete, I made this experimental Deno client.

@kamilogorek
Copy link
Contributor

@KaKi87

Since Deno is also compatible with libraries supporting browsers, this code should also work on Deno, but it doesn't :/

There's one missing API. For now, you can easily work around it with:

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

Sentry.init({
  dsn: "__PUBLIC_DSN__",
  integrations: [
    new Sentry.Integrations.Context({
      app: true,
      os: true,
      device: false, // notice `device` context is off due to missing `os.uptime` API
      culture: true 
    })
  ]
});

You can track the progress of os.uptime here denoland/deno#17850.

@kamilogorek
Copy link
Contributor

It's coming :)

denoland/deno#17179
denoland/std#3052

@KaKi87
Copy link

KaKi87 commented Dec 24, 2022

That works, thanks !

I'm still wondering why the browser SDK didn't work, though 🤔
EDIT: actually, it works using npm:@sentry/node instead of a third-party transpiler.

@HazAT HazAT added the Package: deno Issues related to the Sentry Deno SDK label Jan 25, 2023
@kamilogorek
Copy link
Contributor

With the release of Deno 1.30, our Node SDK should work out-of-the-box now. Would love someone to give it a try and report back if they have any issues! :)

import * as Sentry from "npm:@sentry/node"

Sentry.init({
  dsn: "__PUBLIC_DSN__"
})

@KaKi87
Copy link

KaKi87 commented Jan 30, 2023

Confirmed, thanks !
Still looking forward to native Deno support though.

@jmatsushita
Copy link

Just tried it with deno deploy and https://esm.sh/@sentry/node@7.60.1 and got the following error:

Uncaught SyntaxError: The requested module '/v129/https-proxy-agent@5.0.1/denonext/https-proxy-agent.mjs' does not provide an export named 'HttpsProxyAgent'
    at https://esm.sh/v129/@sentry/node@7.60.1/denonext/node.mjs:8:6347

@getsantry getsantry bot moved this to Waiting for: Product Owner in GitHub Issues with 👀 Oct 5, 2023
@KaKi87
Copy link

KaKi87 commented Oct 5, 2023

I disagree, because even though relying on Deno's support for Node works, it's still suboptimal in terms of performance and size on the Deno end, and in terms of metadata in Sentry's end, as it's full of fake Node metadata and lacking Deno metadata.

Additionally, it doesn't support the "Deno has panicked" error.

Thanks

@alexgleason
Copy link
Contributor

@KaKi87 What would you suggest be done aside from rewriting the whole library?

@KaKi87
Copy link

KaKi87 commented Oct 5, 2023

If that's what was done for Node, then Deno deserves the same work.

@csvn
Copy link

csvn commented Oct 5, 2023

Not sure if this issue covers it, but it would be nice too if Deno was among the list of platforms in Sentry for new projects. For some reason Bun is already supported, but as it's even more identical to NodeJS than Deno, I was a but suprised to see that but not Deno 😅

image

@KaKi87
Copy link

KaKi87 commented Oct 5, 2023

Bun is already supported, but as it's even more identical to NodeJS than Deno

Yes, because Bun was made specifically to replace Node, while Deno is something else entirely, and closer to browser if anything, but more importantly with its own APIs.

@gabeidx
Copy link

gabeidx commented Oct 6, 2023

After the Bun support announcement on X/Twitter, there was a soft commitment to take another look at Deno support in Sentry by @HazAT (with some interest shown by both CEO and CTO of Sentry).

screenshot-2023-10-06T08-45-27+0000@2x

https://twitter.com/DanielGri/status/1704496930364047855

I'm confident it can be a first class integration/support as good as the Node one. Let's just hope it can get prioritized soon 🙏

@eliassjogreen
Copy link

Namespace is taken on the official registry: deno.land/x/sentry@deno

You should be able to contact modules@deno.com and gain access to the name as it seems like it isn't maintained and hasn't been updated for 3+ years.

@AbhiPrasad
Copy link
Member

Hey folks! We do have this on next up on our todo list, we're just finishing up some work we did for automatically detecting ANR/event loop stalls for Node.js and Electron. The ANR feature allows us to detect when the event loop has been blocked for 5+ seconds and generate a stacktrace that points to the line of code causing the block - which means it can be a little complicated to get figured out, hence why we need some extra time there.

Once that is in a good state, we'll move on to Deno!

@timfish
Copy link
Collaborator

timfish commented Oct 8, 2023

Shortly after v1 and before npm support, I created an experimental SDK by converting the existing Sentry code to Deno import syntax.

This weekend I spent a couple of hours working on a branch that adds a Deno SDK using npm specifiers.

We can publish to npm and users can import via:

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

I thought I'd brain-dump my notes here in case anyone subscribed to this issue can offer advice:

  • Deno doesn't appear to publish it's types to npm so I had to copy the 300KB lib.deno.d.ts into the package...

  • The existing rollup config attempts to inject polyfills for things like optional chaining but Deno doesn't like them. Polyfills like this are not required for Deno since it supports modern js features

  • Deno doesn't appear to support resolving workspaces so import { x } from '@sentry/core' is actually loading from npm rather than the local workspace during development.

    • For local development we probably need an import map that maps to the local paths?
    • Once released to npm, Deno supports resolving via package.json
  • Some @sentry/node dependencies @sentry/node > https-proxy-agent > debug have side-effects that access process.env and cause Deno to prompt for env permissions

    • We should probably vendor all the Node SDK dependencies like https-proxy-agent Node SDK - Vendor dependencies  #9199
    • We don't need to use the Node SDK to ship an initial release of a Deno SDK. The browser integrations work.
  • Console and fetch instrumentation via monkey patching doesn't appear to work

    • I had this working in earlier versions of Deno so I guess there have been changes here?
    • When I copy the instrumentation code into my Deno test entry point the monkey patching works. Maybe we can't mutate globals across module boundaries?

@timfish
Copy link
Collaborator

timfish commented Oct 8, 2023

Maybe we can't mutate globals across module boundaries?

denoland/deno - feat(ext/node): properly segregate node globals #19307

This commit makes the globalThis of the entire runtime a semi-proxy.
This proxy returns a different set of globals depending on the caller's
mode. This is not a full proxy, because it is shadowed by "real"
properties on globalThis. This is done to avoid the overhead of a full
proxy for all globalThis operations.

The globals between Deno-mode and Node-mode are now properly segregated.
This means that code running in Deno-mode will not have access to Node's
globals, and vice versa. Deleting a managed global in Deno-mode will
NOT delete the corresponding global in Node-mode, and vice versa.

@timfish
Copy link
Collaborator

timfish commented Oct 9, 2023

Deno doesn't appear to publish it's types to npm so I had to copy the 300KB lib.deno.d.ts into the package...

Turns out this is available from the cli via deno types > so this doesn't need to be kept in source control.

Rather than creating a Deno SDK as an npm module, I have also tried creating it as a pure Deno module that pulls in the Sentry dependencies using npm specifiers (npm:@sentry/core).

This works but I can't work out how to load the local workspace packages (types/core/utils/etc) for local testing. There's no point having the Deno SDK in the JavaScript repository if we can't test it against changes to it's dependencies.

We can't import the TypeScript files directly because Deno only supports node16 resolution and we don't target that with the other packages (missing extensions, etc).

We can import the ESM JavaScript code (ie. ../../../core/esm/index.js) but these obviously don't export the types we need. I've tried referencing the types as per the docs but still get errors.

@lforst
Copy link
Member

lforst commented Oct 9, 2023

@timfish Let's just bundle the SDK + dependencies into one file. There shouldn't be any issues with that besides typing for us to figure out. See #8780

@timfish
Copy link
Collaborator

timfish commented Oct 9, 2023

Let's just bundle the SDK + dependencies into one file

We would be able to publish this to npm, but not to deno.land since that requires that modules are in TypeScript.

It looks like we can access the deno global to instrument console from the node scope like this but it feels a like fragile in the long term:
denoland/deno#20826 (comment)

@eliassjogreen
Copy link

There is nothing mandating that the module is typescript to be published to deno.land, I would suggest you publish it there as JS if that is what makes sense and then reference the typings using the deno (or typescript) type reference comments.

@getsantry getsantry bot moved this to Waiting for: Product Owner in GitHub Issues with 👀 Oct 9, 2023
@timfish timfish mentioned this issue Oct 9, 2023
6 tasks
@timfish
Copy link
Collaborator

timfish commented Oct 10, 2023

There is nothing mandating that the module is typescript to be published to deno.land

Thanks, that's helpful info!

I think the limiting factor might be that we're planning to bundle the code and deno.land expects the source files to be stored in the repository.

We could add a getsentry/sentry-deno repository that we push the build artifacts into specifically for publishing 🤔

https://deno.com/add_module

image

@lforst
Copy link
Member

lforst commented Oct 10, 2023

I think the limiting factor might be that we're planning to bundle the code and deno.land expects the source files to be stored in the repository.

We could add a getsentry/sentry-deno repository that we push the build artifacts into specifically for publishing 🤔

Sounds like a plan to me!

@gabeidx
Copy link

gabeidx commented Oct 12, 2023

Thank you @timfish and @lforst for pushing this over the finish line! ❤️

@AbhiPrasad
Copy link
Member

👀 https://deno.land/x/sentry@7.76.0

Big thanks to @lforst and @timfish for making it happen!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Package: deno Issues related to the Sentry Deno SDK
Projects
Archived in project
Development

Successfully merging a pull request may close this issue.