feat(deno): Add orchestrion deno runtime hook#21451
Conversation
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
isaacs
force-pushed
the
isaacs/deno-orchestrion
branch
from
d6de7a3 to
0ceae6a
Compare
isaacs
force-pushed
the
isaacs/bun-orchestrion
branch
from
39be3da to
b34aa87
Compare
isaacs
force-pushed
the
isaacs/deno-orchestrion
branch
from
0ceae6a to
2011bb7
Compare
![sentry[bot]](https://avatars.githubusercontent.com/in/12637?s=60&v=4){kind=small}
isaacs
force-pushed
the
isaacs/bun-orchestrion
branch
from
b34aa87 to
653fc0e
Compare
isaacs
force-pushed
the
isaacs/deno-orchestrion
branch
from
2011bb7 to
8ab5b64
Compare
JPeer264
left a comment
JPeer264
left a comment
There was a problem hiding this comment.
LGTM, just two minor comments
| ``` | ||
|
|
||
| > [!NOTE] | ||
| > In Deno versions **2.8.0** through **2.8.2**, a bug causes Deno |
There was a problem hiding this comment.
q: Is it worth to mention that it works in these two versions? I would be fine of just saying we are supporting everything after 2.8.3 with the --import option and don't go into much detail - but this is also ok.
There was a problem hiding this comment.
I generally agree that I'd just say "we support deno 2.8.3 and above" and not really go into details for anything below that?
| * orchestrion runtime hook (`@sentry/deno/import`) needs to transform libraries | ||
| * like `mysql` so they publish to their tracing channels. | ||
| */ | ||
| export const MODULE_REGISTER_HOOKS_SUPPORTED = gte(2, 8, 0); |
There was a problem hiding this comment.
q/l: Connected to the other comment. Should we actually start supporting it form 2.8.3?
There was a problem hiding this comment.
I mean, it's fine, I guess? Probably no one's going to be using Deno 2.8.0, but they did make a big announcement about it when it came out, and haven't been as noisy about the patches, so it's possible someone upgraded right away, but then is lagging behind, I guess?
There was a problem hiding this comment.
Ok if they made big announcements it might be better as is.
isaacs
force-pushed
the
isaacs/bun-orchestrion
branch
from
653fc0e to
8743326
Compare
isaacs
force-pushed
the
isaacs/deno-orchestrion
branch
from
8ab5b64 to
4f35ab9
Compare
isaacs
force-pushed
the
isaacs/bun-orchestrion
branch
from
b5a794a to
ac38662
Compare
isaacs
force-pushed
the
isaacs/deno-orchestrion
branch
from
4f35ab9 to
e54572f
Compare
size-limit report 📦
|
isaacs
force-pushed
the
isaacs/bun-orchestrion
branch
from
ac38662 to
0cfd982
Compare
isaacs
force-pushed
the
isaacs/deno-orchestrion
branch
from
e54572f to
21830a7
Compare
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 1 potential issue.
❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.
Reviewed by Cursor Bugbot for commit 21830a7. Configure here.
isaacs
force-pushed
the
isaacs/bun-orchestrion
branch
from
0cfd982 to
ccbd1b2
Compare
Use the orchestrion loader hook defined in server-utils, and create a loader for Deno that detects the presence of the hooks, and instruments the channels added to the mysql module. Documentation added to call out the caveat of usage in Deno v2.8.0 through 2.8.2, which is fixed in 2.8.3.
isaacs
force-pushed
the
isaacs/deno-orchestrion
branch
from
21830a7 to
1caca4f
Compare
Use the orchestrion loader hook defined in server-utils, and create a loader for Deno that detects the presence of the hooks, and instruments the channels added to the mysql module. Documentation added to call out the caveat of usage in Deno v2.8.0 through 2.8.2, which is fixed in 2.8.3.
isaacs
force-pushed
the
isaacs/deno-orchestrion
branch
from
1caca4f to
4be4406
Compare
| "@sentry/server-utils": "10.58.0" | ||
| }, | ||
| "devDependencies": { | ||
| "mysql": "^2.18.1" |
There was a problem hiding this comment.
l/m: I would generally try to avoid adding these things as dev dependencies to the packages, can we instead somehow contain this to a separate deno-integration-tests package or similar? It is also fine to merge this here for now but if we follow this with all the other instrumentation this will become unwieldly IMHO
There was a problem hiding this comment.
Yeah, that's a good point. I also added a real E2E test, maybe we can just remove the synchronous test and let that be the guard?
Use the orchestrion loader hook defined in server-utils, and create a loader for Deno that detects the presence of the hooks, and instruments the channels added to the mysql module. Documentation added to call out the caveat of usage in Deno v2.8.0 through 2.8.2, which is fixed in 2.8.3.
isaacs
force-pushed
the
isaacs/deno-orchestrion
branch
from
4be4406 to
0c3dd87
Compare
isaacs
force-pushed
the
isaacs/bun-orchestrion
branch
from
ccbd1b2 to
33c961c
Compare
Use the orchestrion loader hook defined in server-utils, and create a loader for Deno that detects the presence of the hooks, and instruments the channels added to the mysql module. Documentation added to call out the caveat of usage in Deno v2.8.0 through 2.8.2, which is fixed in 2.8.3.
isaacs
force-pushed
the
isaacs/deno-orchestrion
branch
from
0c3dd87 to
af5a33e
Compare
Use the orchestrion loader hook defined in server-utils, and create a loader for Deno that detects the presence of the hooks, and instruments the channels added to the mysql module. Documentation added to call out the caveat of usage in Deno v2.8.0 through 2.8.2, which is fixed in 2.8.3.
isaacs
force-pushed
the
isaacs/deno-orchestrion
branch
from
af5a33e to
f156e1c
Compare
isaacs
force-pushed
the
isaacs/bun-orchestrion
branch
3 times, most recently
from
942cbc7 to
b7d3bf4
Compare
Use the orchestrion loader hook defined in server-utils, and create a loader for Deno that detects the presence of the hooks, and instruments the channels added to the mysql module. Documentation added to call out the caveat of usage in Deno v2.8.0 through 2.8.2, which is fixed in 2.8.3.
isaacs
force-pushed
the
isaacs/deno-orchestrion
branch
from
f156e1c to
5b20801
Compare
isaacs
force-pushed
the
isaacs/bun-orchestrion
branch
from
b7d3bf4 to
5b4cb48
Compare
Use the orchestrion loader hook defined in server-utils, and create a loader for Deno that detects the presence of the hooks, and instruments the channels added to the mysql module. Documentation added to call out the caveat of usage in Deno v2.8.0 through 2.8.2, which is fixed in 2.8.3.
isaacs
force-pushed
the
isaacs/deno-orchestrion
branch
from
5b20801 to
4793bdc
Compare
isaacs
force-pushed
the
isaacs/bun-orchestrion
branch
from
5b4cb48 to
cfe1a03
Compare
Use the orchestrion loader hook defined in server-utils, and create a loader for Deno that detects the presence of the hooks, and instruments the channels added to the mysql module. Documentation added to call out the caveat of usage in Deno v2.8.0 through 2.8.2, which is fixed in 2.8.3.
isaacs
force-pushed
the
isaacs/deno-orchestrion
branch
from
4793bdc to
6b86f67
Compare
isaacs
force-pushed
the
isaacs/bun-orchestrion
branch
from
cfe1a03 to
568911f
Compare
Use the orchestrion loader hook defined in server-utils, and create a loader for Deno that detects the presence of the hooks, and instruments the channels added to the mysql module. Documentation added to call out the caveat of usage in Deno v2.8.0 through 2.8.2, which is fixed in 2.8.3.
isaacs
force-pushed
the
isaacs/deno-orchestrion
branch
from
6b86f67 to
1714284
Compare
| * | ||
| * @module | ||
| */ | ||
| import '@sentry/server-utils/orchestrion/import-hook'; |
There was a problem hiding this comment.
Bug: Accessing process.versions.node in a Deno environment throws a ReferenceError because process is not defined, causing the application to crash on startup.
Severity: CRITICAL
Suggested Fix
Guard the access to process.versions.node with a check to ensure the process object and its properties exist before being accessed, for example, by using typeof process !== 'undefined' && process.versions && process.versions.node. This will prevent the ReferenceError in non-Node.js environments like Deno.
Prompt for AI Agent
Review the code at the location below. A potential bug has been identified by an AI
agent. Verify if this is a real issue. If it is, propose a fix; if not, explain why it's
not valid.
Location: packages/deno/src/import.mjs#L31
Potential issue: The function `registerDiagnosticsChannelInjection()` accesses
`process.versions.node` to determine the Node.js version. When this code is executed in
a Deno environment, which is triggered by using `@sentry/deno/import`, it throws a
`ReferenceError` because the `process` global is not defined. This error occurs during
module loading, outside of any `try-catch` blocks intended to handle instrumentation
failures. As a result, any Deno application using `--import=@sentry/deno/import` will
crash immediately on startup.
Also affects:
packages/server-utils/src/orchestrion/runtime/register.ts
Use the orchestrion loader hook defined in server-utils, and create a loader for Deno that detects the presence of the hooks, and instruments the channels added to the mysql module. Documentation added to call out the caveat of usage in Deno v2.8.0 through 2.8.2, which is fixed in 2.8.3.
isaacs
force-pushed
the
isaacs/deno-orchestrion
branch
from
1714284 to
79c1076
Compare
isaacs
force-pushed
the
isaacs/bun-orchestrion
branch
from
568911f to
69a65aa
Compare
|
Semgrep found 1 Risk: Affected versions of esbuild are vulnerable to Download of Code Without Integrity Check / Untrusted Search Path. esbuild's Deno distribution module ( Manual Review Advice: A vulnerability from this advisory is reachable if you invoke the esbuild Deno module directly as a CLI tool (e.g. Fix: Upgrade this library to at least version 0.28.1 at sentry-javascript/yarn.lock:15987. Reference(s): GHSA-gv7w-rqvm-qjhr |
3 participants
Use the orchestrion loader hook defined in server-utils, and create a loader for Deno that detects the presence of the hooks, and instruments the channels added to the mysql module.
Documentation added to call out the caveat of usage in Deno v2.8.0 through 2.8.2, which is fixed in 2.8.3.