fix(ext/otel): remove panicking unwraps in telemetry code#32557
Merged
bartlomieju merged 1 commit intodenoland:mainfrom Mar 9, 2026
Merged
fix(ext/otel): remove panicking unwraps in telemetry code#32557bartlomieju merged 1 commit intodenoland:mainfrom
bartlomieju merged 1 commit intodenoland:mainfrom
Conversation
Replace `.unwrap()` calls on `OTEL_GLOBALS.get()` and isolate slot accesses with graceful error handling. These unwraps could panic during watch mode restarts when the isolate is being torn down (GC callbacks fire after slots are cleaned up) or if OTEL globals are not yet initialized. - GC prologue/epilogue callbacks: return early if isolate slot is gone - op_otel_enable_isolate_metrics: return early if OTEL_GLOBALS unset - op_otel_collect_isolate_metrics: return early if slot is gone - OtelTracer/OtelMeter constructors: return JS errors instead of panicking Refs: denoland#29478 Refs: denoland#30567 Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
kajukitli
approved these changes
Mar 7, 2026
Contributor
kajukitli
left a comment
There was a problem hiding this comment.
lgtm — good defensive fix for watch mode crashes
all the changes are straightforward:
.unwrap()→.ok_or_else()with proper JS error for ops.unwrap()→ early return for GC callbacks (can't propagate errors from V8 callbacks anyway)- isolate slot access now guards against teardown race conditions
the GC callbacks returning early is the right approach since they're called from V8 and can't meaningfully report errors
littledivy
approved these changes
Mar 9, 2026
This was referenced Mar 9, 2026
Closed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
.unwrap()calls onOTEL_GLOBALS.get()and isolate slot accesseswith graceful error handling throughout
ext/telemetry/lib.rs(can happen during isolate teardown in watch mode restarts)
op_otel_enable_isolate_metrics/op_otel_collect_isolate_metricsreturnearly if globals or slots are not available
OtelTracer::builtin(),OtelTracer::start_span(),OtelTracer::start_span_foreign(), andOtelMeter::new()return JS errorsinstead of panicking
Context
These unwraps could panic in watch mode when:
Refs: #29478
Refs: #30567
Test plan
OTEL_DENO=true deno run --watch+ rapid file changes shows no panics