fix: replace uuid dependency with node:crypto.randomUUID (critical crash fix)#24
Merged
Conversation
uuid@12.0.0 dropped CommonJS support entirely (ESM-only since then).
sovdev-logger's OTel dependency sweep bumped uuid 9.0.1 -> 14.0.1 without
catching this -- the compiled dist/logger.js's static require('uuid')
now throws ERR_REQUIRE_ESM immediately on load, crashing every consumer,
not just an edge case. No security tradeoff either: npm audit shows zero
advisories for uuid at any version, so the original bump wasn't fixing
a vulnerability.
crypto.randomUUID() (Node builtin since v14.17, this project already
requires >=22.0.0) is a drop-in replacement for the plain v4 UUIDs used
here (event_id, temp_trace_id, session_id) -- it's literally what
@opentelemetry/resources' own ServiceInstanceIdDetector uses internally
for the same purpose. Removes the uuid/@types/uuid dependency entirely
rather than just pinning back to a CJS-compatible major, so this can't
regress again from a future uuid bump.
Added a postbuild smoke test (node -e "require('./dist/index.js')") so
a require()-breaking regression like this fails the build immediately
instead of only surfacing when a real consumer hits the crash.
Verified end-to-end against real Grafana Cloud data (write + read-back
via sovdev-selftest), not just a local build check.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
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
1 participant
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
uuid@12.0.0+ dropped CommonJS support entirely. The earlier OTel dependency sweep bumpeduuid9.0.1→14.0.1, and the compileddist/logger.js's staticrequire('uuid')now throwsERR_REQUIRE_ESMimmediately on load — crashing every consumer of the published package, not an edge case.node:crypto's built-inrandomUUID()— a drop-in replacement for the plain v4 UUIDs used here (event_id,temp_trace_id,session_id). This is literally what@opentelemetry/resources' ownServiceInstanceIdDetectoruses internally for the same purpose.uuid/@types/uuiddependency entirely (not just pinned back) — no security tradeoff,npm auditshows zero advisories foruuidat any version, so the original bump wasn't fixing a vulnerability.postbuildsmoke test (node -e "require('./dist/index.js')") so arequire()-breaking regression like this fails the build immediately instead of only surfacing when a real consumer hits the crash.1.0.1→1.0.2(patch — internal dependency fix, no public API change).Test plan
npm run build— clean, postbuild smoke test passesnode -e "require('./dist/logger.js')"— confirmed crash before fix, confirmed clean aftersovdev-selftest: write-log, write-metric, read-log, read-metric all pass, using a real external consumer's (ollacrm) own credentialsnpm audit— 0 vulnerabilities after removinguuid🤖 Generated with Claude Code