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

fix(browser): update Request checks to be valid in Deno #5630

Merged
merged 1 commit into from Aug 29, 2022

Conversation

GJZwiers
Copy link
Contributor

@GJZwiers GJZwiers commented Aug 24, 2022

In Deno @sentry/browser is almost usable, but the check new Request('') fails silently because Deno throws a TypeError: invalid URL that goes to a catch block. This changes the Request call to include a valid URL http://www.example.com so that @sentry/browser can be imported in Deno without this issue.

@AbhiPrasad
Copy link
Member

Hey @GJZwiers, thanks for the contribution! How are you using Sentry with deno?

@GJZwiers
Copy link
Contributor Author

GJZwiers commented Aug 24, 2022

Hey @GJZwiers, thanks for the contribution! How are you using Sentry with deno?

Hello! Recently I tried to port the browser package to Deno, which works well enough for sending events to Sentry (but it's difficult to maintain for me). A lot of Deno users also use CDNs to import code, for example there is another Sentry port in the making. But in the next release of Deno there will be more support for importing npm packages, which will look something like this:

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

I believe most of the browser APIs used in sentry are now also supported in Deno, so I think it should be possible, maybe with a few small tweaks, to use Sentry in Deno this way.

Copy link
Member

@AbhiPrasad AbhiPrasad left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Awesome, this sounds reasonable to me, cc @timfish

@AbhiPrasad AbhiPrasad merged commit 14a7a45 into getsentry:master Aug 29, 2022
@GJZwiers GJZwiers deleted the update_request_check branch August 29, 2022 20:07
@timfish
Copy link
Collaborator

timfish commented Aug 29, 2022

I've just had a chance to try out the Deno support for npm packages. It looks like npm packages support currently requires the --unstable flag so the behaviour is likely to improve/change over time.

Whereas @sentry/browser might be usable at some point, there are still outstanding issues:

  • Merely referencing process in @sentry/utils causes some node shims to be automatically imported which then prompt for access to the NODE_DEBUG environment variable. There's currently no way to prevent this
  • If I enable debug: true I get Uncaught TypeError: Cannot read properties of undefined (reading 'log') from @sentry/utils logger/consoleSandbox
  • I haven't managed to get the npm browser package to send events to Sentry from Deno yet

There are also some other key features which I suspect will always need to be specifically coded for Deno:

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

Successfully merging this pull request may close these issues.

None yet

3 participants