diff --git a/js/src/auto-instrumentations/import-in-the-middle/create-hook.mts b/js/src/auto-instrumentations/import-in-the-middle/create-hook.mts index 26994c5a9..a631cdcd8 100644 --- a/js/src/auto-instrumentations/import-in-the-middle/create-hook.mts +++ b/js/src/auto-instrumentations/import-in-the-middle/create-hook.mts @@ -16,14 +16,10 @@ import type { ResolveOperation, ResolveResult, } from "./lib/io.mjs"; -import type { register as registerGeneratedWrapper } from "./lib/register.mjs"; - -const generatedWrapperRegister: typeof registerGeneratedWrapper | undefined = - undefined; -void generatedWrapperRegister; const specifiers = new Map(); const isWin = process.platform === "win32"; +const IITM_QUERY_PARAM = "braintrust_iitm"; // FIXME: Typescript extensions are added temporarily until we find a better // way of supporting arbitrary extensions @@ -40,9 +36,6 @@ const [, NODE_MAJOR, NODE_MINOR, NODE_PATCH] = process.versions.node type LoaderMeta = { url: string }; type HookData = { addHookMessagePort?: MessagePort; - exclude?: never; - include?: readonly string[]; - shouldInclude?: never; }; type AsyncLoadFunction = ( url: string, @@ -57,7 +50,6 @@ type SyncResolveFunction = ( specifier: string, context: LoaderContext, ) => ResolveResult; -type ResolveFunction = AsyncResolveFunction | SyncResolveFunction; type SetterMap = Map; interface ImportInTheMiddleHook { @@ -110,12 +102,12 @@ export function supportsSyncHooks(): boolean { } function hasIitm(url: unknown): boolean { - // Fast path: avoid URL parsing on the hot path when there's clearly no iitm. - if (typeof url !== "string" || url.indexOf("iitm") === -1) { + // Fast path: avoid URL parsing on the hot path when our marker is absent. + if (typeof url !== "string" || url.indexOf(IITM_QUERY_PARAM) === -1) { return false; } try { - return new URL(url).searchParams.has("iitm"); + return new URL(url).searchParams.has(IITM_QUERY_PARAM); } catch { return false; } @@ -129,7 +121,7 @@ function isIitm(url: string, meta: LoaderMeta): boolean { function deleteIitm(url: string): string { // Fast path: avoid URL parsing / try-catch on bare specifiers and normal file URLs. - if (typeof url !== "string" || url.indexOf("iitm") === -1) { + if (typeof url !== "string" || url.indexOf(IITM_QUERY_PARAM) === -1) { return url; } let resultUrl: string; @@ -137,8 +129,8 @@ function deleteIitm(url: string): string { try { Error.stackTraceLimit = 0; const urlObj = new URL(url); - if (urlObj.searchParams.has("iitm")) { - urlObj.searchParams.delete("iitm"); + if (urlObj.searchParams.has(IITM_QUERY_PARAM)) { + urlObj.searchParams.delete(IITM_QUERY_PARAM); resultUrl = urlObj.href; if (resultUrl.startsWith("file:///node:")) { resultUrl = resultUrl.replace("file:///", ""); @@ -371,17 +363,18 @@ function* processModule({ function addIitm(url: string): string { const urlObj = new URL(url); - urlObj.searchParams.set("iitm", "true"); + urlObj.searchParams.set(IITM_QUERY_PARAM, "true"); return urlObj.href; } export function createHook(meta: LoaderMeta): ImportInTheMiddleHook { - let cachedResolve: ResolveFunction | undefined; + let cachedAsyncResolve: AsyncResolveFunction | undefined; + let cachedSyncResolve: SyncResolveFunction | undefined; const iitmURL = new URL( meta.url.endsWith(".mts") ? "lib/register.mts" : "lib/register.mjs", meta.url, ).toString(); - const includeModules = new Set(); + const loaderThreadHookedModules = new Set(); // Track CJS module URLs that IITM has wrapped. On Node 24+, CJS modules loaded // via loadCJSModule (in an ESM import chain) have their require() calls for @@ -391,7 +384,7 @@ export function createHook(meta: LoaderMeta): ImportInTheMiddleHook { // patterns like `class App extends require('events') {}`. const cjsInIitmChain = new Set(); - function addExplicitIncludes(modules: unknown): void { + function addExplicitHookModules(modules: unknown): void { if (!Array.isArray(modules)) { return; } @@ -402,16 +395,18 @@ export function createHook(meta: LoaderMeta): ImportInTheMiddleHook { `Braintrust import-in-the-middle only supports string module names. Invalid entry: ${inspect(each)}`, ); } - includeModules.add(each); + loaderThreadHookedModules.add(each); if (!each.startsWith("node:") && builtinModules.includes(each)) { - includeModules.add(`node:${each}`); + loaderThreadHookedModules.add(`node:${each}`); } } } - function defaultShouldInclude(url: string, specifier: string): boolean { + function shouldWrapExplicitHook(url: string, specifier: string): boolean { const modules = - includeModules.size > 0 ? includeModules : registerState.hookedModules; + loaderThreadHookedModules.size > 0 + ? loaderThreadHookedModules + : registerState.hookedModules; if (!modules || modules.size === 0) { return false; } @@ -445,19 +440,11 @@ export function createHook(meta: LoaderMeta): ImportInTheMiddleHook { } function applyOptions(data: HookData): void { - if (data.exclude || data.shouldInclude) { - throw new Error( - "Braintrust import-in-the-middle only supports explicit Hook([...]) module interception", - ); - } - - addExplicitIncludes(data.include); - const { addHookMessagePort } = data; if (addHookMessagePort) { addHookMessagePort .on("message", (modules: unknown) => { - addExplicitIncludes(modules); + addExplicitHookModules(modules); addHookMessagePort.postMessage("ack"); }) .unref(); @@ -466,15 +453,15 @@ export function createHook(meta: LoaderMeta): ImportInTheMiddleHook { async function initialize(data?: HookData): Promise { const globalState = globalThis as typeof globalThis & { - __import_in_the_middle_initialized__?: boolean; + __braintrust_import_in_the_middle_initialized__?: boolean; }; - if (globalState.__import_in_the_middle_initialized__) { + if (globalState.__braintrust_import_in_the_middle_initialized__) { process.emitWarning( - "The 'import-in-the-middle' hook has already been initialized", + "The Braintrust import-in-the-middle hook has already been initialized", ); } - globalState.__import_in_the_middle_initialized__ = true; + globalState.__braintrust_import_in_the_middle_initialized__ = true; if (data) { applyOptions(data); @@ -520,7 +507,7 @@ export function createHook(meta: LoaderMeta): ImportInTheMiddleHook { return result; } - if (!defaultShouldInclude(result.url, specifier)) { + if (!shouldWrapExplicitHook(result.url, specifier)) { return result; } @@ -579,7 +566,7 @@ export function createHook(meta: LoaderMeta): ImportInTheMiddleHook { context: LoaderContext, parentResolve: AsyncResolveFunction, ): Promise { - cachedResolve = parentResolve; + cachedAsyncResolve = parentResolve; // See https://github.com/nodejs/import-in-the-middle/pull/76. if (specifier === iitmURL) { @@ -611,7 +598,7 @@ export function createHook(meta: LoaderMeta): ImportInTheMiddleHook { context: LoaderContext, nextResolve: SyncResolveFunction, ): ResolveResult { - cachedResolve = nextResolve; + cachedSyncResolve = nextResolve; if (specifier === iitmURL) { return { @@ -639,7 +626,7 @@ export function createHook(meta: LoaderMeta): ImportInTheMiddleHook { originalSpecifier: string | undefined, ): string { return ` -import { register } from '${iitmURL}' +import registerState from '${iitmURL}' import * as namespace from ${JSON.stringify(realUrl)} // Mimic a Module object (https://tc39.es/ecma262/#sec-module-namespace-objects). @@ -705,7 +692,7 @@ if (__pending.length > 0) { }) } -register(${JSON.stringify(realUrl)}, _, set, get, ${JSON.stringify(originalSpecifier)}) +registerState.register(${JSON.stringify(realUrl)}, _, set, get, ${JSON.stringify(originalSpecifier)}) `; } @@ -753,7 +740,7 @@ register(${JSON.stringify(realUrl)}, _, set, get, ${JSON.stringify(originalSpeci const originalSpecifier = specifiers.get(realUrl); try { - const resolveForWrap = cachedResolve; + const resolveForWrap = cachedAsyncResolve; const setters = await driveAsync( processModule({ srcUrl: realUrl, context }), { @@ -797,7 +784,7 @@ register(${JSON.stringify(realUrl)}, _, set, get, ${JSON.stringify(originalSpeci const setters = driveSync(processModule({ srcUrl: realUrl, context }), { load: (loadUrl, loadContext) => nextLoad(loadUrl, loadContext) as LoadResult, - resolve: cachedResolve as SyncResolveFunction | undefined, + resolve: cachedSyncResolve, }); return { source: onWrapSuccess(realUrl, context, originalSpecifier, setters), diff --git a/js/src/auto-instrumentations/import-in-the-middle/index.mts b/js/src/auto-instrumentations/import-in-the-middle/index.mts index fc33d5722..6c8d5ae7a 100644 --- a/js/src/auto-instrumentations/import-in-the-middle/index.mts +++ b/js/src/auto-instrumentations/import-in-the-middle/index.mts @@ -5,7 +5,7 @@ import moduleDetailsFromPath from "module-details-from-path"; import { isBuiltin } from "node:module"; import { fileURLToPath } from "node:url"; -import { MessageChannel, type MessagePort } from "node:worker_threads"; +import { MessageChannel } from "node:worker_threads"; import registerState, { type ImportHook, type Namespace, @@ -19,52 +19,13 @@ const { toHook, } = registerState; -type HookFn = ( - exported: Exported, - name: string, - baseDir?: string, -) => Result; - -interface Hook { - unhook(): void; -} - -interface HookConstructor { - new ( - modules: readonly string[], - hookFn: HookFn, - ): Hook; - ( - modules: readonly string[], - hookFn: HookFn, - ): Hook; -} - -type HookRegisterData = { - addHookMessagePort: MessagePort; - include: string[]; -}; - -type CreateAddHookMessageChannelReturn = { - addHookMessagePort: MessagePort; - waitForAllMessagesAcknowledged: () => Promise; - registerOptions: { - data: HookRegisterData; - transferList: [MessagePort]; - }; -}; +type HookFn = (exported: Namespace, name: string, baseDir?: string) => unknown; interface HookInstance { _iitmHook: ImportHook; _modules: string[]; } -interface HookImplementationConstructor { - new (modules: unknown, hookFn: unknown): HookInstance; - (modules: unknown, hookFn: unknown): HookInstance; - prototype: HookInstance & { unhook(): void }; -} - let sendModulesToLoader: ((modules: string[]) => void) | undefined; function addHook(hook: ImportHook): void { @@ -82,7 +43,7 @@ function removeHook(hook: ImportHook): void { } function callHookFn( - hookFn: HookFn, + hookFn: HookFn, namespace: Namespace, name: string, baseDir?: string, @@ -93,26 +54,6 @@ function callHookFn( } } -function normalizeModules(modules: unknown): string[] { - if (!Array.isArray(modules) || modules.length === 0) { - throw new TypeError( - "Braintrust import-in-the-middle requires a non-empty modules array", - ); - } - - const normalized: string[] = []; - for (const each of modules) { - if (typeof each !== "string") { - throw new TypeError( - "Braintrust import-in-the-middle only supports string module names or file URLs", - ); - } - normalized.push(each); - } - - return normalized; -} - function moduleMatches( matchArg: string, name: string, @@ -138,7 +79,7 @@ function moduleMatches( return matchArg === name && baseDir.endsWith(String(specifiers.get(loadUrl))); } -export function createAddHookMessageChannel(): CreateAddHookMessageChannelReturn { +export function createAddHookMessageChannel() { const { port1, port2 } = new MessageChannel(); let pendingAckCount = 0; let resolveFn: (() => void) | undefined; @@ -174,11 +115,10 @@ export function createAddHookMessageChannel(): CreateAddHookMessageChannelReturn } const addHookMessagePort = port2; - const registerOptions: CreateAddHookMessageChannelReturn["registerOptions"] = - { - data: { addHookMessagePort, include: [] as string[] }, - transferList: [addHookMessagePort], - }; + const registerOptions = { + data: { addHookMessagePort }, + transferList: [addHookMessagePort], + }; return { registerOptions, @@ -187,77 +127,68 @@ export function createAddHookMessageChannel(): CreateAddHookMessageChannelReturn }; } -const HookImpl: HookImplementationConstructor = function ( - this: HookInstance | undefined, - modules: unknown, - hookFn: unknown, -): HookInstance | void { - if (!this || !(this instanceof HookImpl)) { - return new HookImpl(modules, hookFn); - } - - const normalizedModules = normalizeModules(modules); - if (typeof hookFn !== "function") { - throw new TypeError( - "Braintrust import-in-the-middle requires a hook function", - ); - } - const importHookFn = hookFn as HookFn; +class Hook implements HookInstance { + _iitmHook: ImportHook; + _modules: string[]; - addHookedModules(normalizedModules); - if (sendModulesToLoader) { - sendModulesToLoader(normalizedModules); - } + constructor(modules: readonly string[], hookFn: HookFn) { + const normalizedModules = [...modules]; - this._modules = normalizedModules; - this._iitmHook = (name, namespace, specifier) => { - const loadUrl = name; - let filePath: string | undefined; - let baseDir: string | undefined; + addHookedModules(normalizedModules); + if (sendModulesToLoader) { + sendModulesToLoader(normalizedModules); + } - if (loadUrl.startsWith("node:")) { - const unprefixed = name.slice(5); - if (isBuiltin(unprefixed)) { - name = unprefixed; - } - } else if (loadUrl.startsWith("file://")) { - const stackTraceLimit = Error.stackTraceLimit; - Error.stackTraceLimit = 0; - try { - filePath = fileURLToPath(name); - name = filePath; - } catch {} - Error.stackTraceLimit = stackTraceLimit; + this._modules = normalizedModules; + this._iitmHook = (name, namespace, specifier) => { + const loadUrl = name; + let filePath: string | undefined; + let baseDir: string | undefined; - if (filePath) { - const details = moduleDetailsFromPath(filePath); - if (details) { - name = details.name; - baseDir = details.basedir; + if (loadUrl.startsWith("node:")) { + const unprefixed = name.slice(5); + if (isBuiltin(unprefixed)) { + name = unprefixed; + } + } else if (loadUrl.startsWith("file://")) { + const stackTraceLimit = Error.stackTraceLimit; + Error.stackTraceLimit = 0; + try { + filePath = fileURLToPath(name); + name = filePath; + } catch {} + Error.stackTraceLimit = stackTraceLimit; + + if (filePath) { + const details = moduleDetailsFromPath(filePath); + if (details) { + name = details.name; + baseDir = details.basedir; + } } } - } - for (const matchArg of normalizedModules) { - if ( - moduleMatches(matchArg, name, filePath, specifier, baseDir, loadUrl) - ) { - callHookFn( - importHookFn, - namespace, - filePath && matchArg === filePath ? filePath : matchArg, - baseDir, - ); + for (const matchArg of normalizedModules) { + if ( + moduleMatches(matchArg, name, filePath, specifier, baseDir, loadUrl) + ) { + callHookFn( + hookFn, + namespace, + filePath && matchArg === filePath ? filePath : matchArg, + baseDir, + ); + } } - } - }; + }; - addHook(this._iitmHook); -} as HookImplementationConstructor; + addHook(this._iitmHook); + } -HookImpl.prototype.unhook = function (this: HookInstance): void { - removeHook(this._iitmHook); - deleteHookedModules(this._modules); -}; + unhook(): void { + removeHook(this._iitmHook); + deleteHookedModules(this._modules); + } +} -export const Hook = HookImpl as unknown as HookConstructor; +export { Hook }; diff --git a/js/src/auto-instrumentations/import-in-the-middle/lib/get-exports.mts b/js/src/auto-instrumentations/import-in-the-middle/lib/get-exports.mts index 2f066c46b..2b9854ec2 100644 --- a/js/src/auto-instrumentations/import-in-the-middle/lib/get-exports.mts +++ b/js/src/auto-instrumentations/import-in-the-middle/lib/get-exports.mts @@ -30,11 +30,13 @@ function ensureParserInitialized() { type NodeRequire = ReturnType; type ExportGenerator = Generator, LoadResult>; -type PackageImportObject = { - default?: string; - import?: { default?: string; node?: string }; - node?: string; - require?: { default?: string; node?: string }; +type PackageImportBranch = { + default?: unknown; + node?: unknown; +}; +type PackageImportObject = PackageImportBranch & { + import?: unknown; + require?: unknown; }; function addDefault(arr: Iterable): Set { @@ -169,7 +171,7 @@ let require: NodeRequire | undefined; function getRequire(): NodeRequire { if (!require) { - require = createRequire(import.meta.url); + require = createRequire(pathToFileURL(process.execPath)); } return require; } @@ -178,6 +180,46 @@ function isRecord(value: unknown): value is Record { return value !== null && typeof value === "object"; } +function getStringProperty( + value: Record, + property: string, +): string | undefined { + const result = value[property]; + return typeof result === "string" ? result : undefined; +} + +function getImportBranch(value: unknown): PackageImportBranch | undefined { + return isRecord(value) ? value : undefined; +} + +function getPackageImportTarget(imports: unknown): string | undefined { + if (typeof imports === "string") { + return imports; + } + + if (!isRecord(imports)) { + return undefined; + } + + const conditionalImports = imports as PackageImportObject; + const requireExport = getImportBranch(conditionalImports.require); + const importExport = getImportBranch(conditionalImports.import); + + if (requireExport || importExport) { + return ( + (requireExport && getStringProperty(requireExport, "node")) || + (requireExport && getStringProperty(requireExport, "default")) || + (importExport && getStringProperty(importExport, "node")) || + (importExport && getStringProperty(importExport, "default")) + ); + } + + return ( + getStringProperty(conditionalImports, "node") || + getStringProperty(conditionalImports, "default") + ); +} + // Returns a builtin's exports object. `process.getBuiltinModule` (Node >= // 20.16 / >= 22.3) bypasses registered loader hooks; `require` does not. Under // the in-thread `module.registerHooks` loader a plain `require(name)` here @@ -234,31 +276,14 @@ function resolvePackageImports( if (existsSync(packageJsonPath)) { const packageJson = JSON.parse( readFileSync(packageJsonPath, "utf8"), - ) as { imports?: Record }; - if (packageJson.imports && packageJson.imports[specifier]) { - const imports = packageJson.imports[specifier]; - - // Look for path inside packageJson - let resolvedExport: string | undefined; - if (isRecord(imports)) { - const conditionalImports = imports as PackageImportObject; - const requireExport = conditionalImports.require; - const importExport = conditionalImports.import; - // look for the possibility of require and import which is standard for CJS/ESM - if (requireExport || importExport) { - // trying to resolve based on order of importance - resolvedExport = - requireExport?.node || - requireExport?.default || - importExport?.node || - importExport?.default; - } else if (conditionalImports.node || conditionalImports.default) { - resolvedExport = - conditionalImports.node || conditionalImports.default; - } - } else if (typeof imports === "string") { - resolvedExport = imports; - } + ) as unknown; + const packageImports = + isRecord(packageJson) && isRecord(packageJson.imports) + ? packageJson.imports[specifier] + : undefined; + + if (packageImports) { + const resolvedExport = getPackageImportTarget(packageImports); if (resolvedExport) { const url = resolvedExport.startsWith(".") diff --git a/js/src/auto-instrumentations/import-in-the-middle/lib/register.mts b/js/src/auto-instrumentations/import-in-the-middle/lib/register.mts index 8d1b26ac7..7da542679 100644 --- a/js/src/auto-instrumentations/import-in-the-middle/lib/register.mts +++ b/js/src/auto-instrumentations/import-in-the-middle/lib/register.mts @@ -85,7 +85,7 @@ const proxyHandler: ProxyHandler = { }, }; -export function register( +function register( name: string, namespace: Namespace, set: Record, diff --git a/js/src/auto-instrumentations/require-in-the-middle/index.ts b/js/src/auto-instrumentations/require-in-the-middle/index.ts index 3769e8b70..416e22644 100644 --- a/js/src/auto-instrumentations/require-in-the-middle/index.ts +++ b/js/src/auto-instrumentations/require-in-the-middle/index.ts @@ -2,12 +2,11 @@ import Module from "node:module"; import moduleDetailsFromPath from "module-details-from-path"; import path from "node:path"; -type OnRequireFn = ( - exports: Exports, +type OnRequireFn = ( + exports: unknown, name: string, basedir?: string, -) => PatchedExports; - +) => unknown; type GetBuiltinModuleFn = (this: unknown, id: string) => unknown; type ProcessWithGetBuiltinModule = Omit & { getBuiltinModule?: GetBuiltinModuleFn; @@ -18,44 +17,8 @@ type ModuleWithInternals = typeof Module & { type CachedModule = NodeJS.Module & Record; type HookedRequire = (this: NodeJS.Module, id: string) => unknown; -interface HookInstance { - _cache: ExportsCache; - _getBuiltinModule?: GetBuiltinModuleFn; - _origGetBuiltinModule?: GetBuiltinModuleFn; - _origRequire: HookedRequire; - _require: HookedRequire; - _unhooked: boolean; -} - -interface HookImplementationConstructor { - new (modules: unknown, onrequire: unknown): HookInstance; - (modules: unknown, onrequire: unknown): HookInstance; - prototype: HookInstance & { unhook(): void }; -} - -let builtinModules: Set | undefined; - const ModuleInternals = Module as ModuleWithInternals; -let isCore: (moduleName: string) => boolean; -if (Module.isBuiltin) { - isCore = Module.isBuiltin; -} else if (Module.builtinModules) { - isCore = (moduleName: string) => { - if (moduleName.startsWith("node:")) { - return true; - } - - if (builtinModules === undefined) { - builtinModules = new Set(Module.builtinModules); - } - - return builtinModules.has(moduleName); - }; -} else { - throw new Error( - "Braintrust require-in-the-middle requires Node.js >=v9.3.0 or >=v8.10.0", - ); -} +const isCore = Module.isBuiltin; const normalize = /([/\\]index)?(\.js|\.cjs)?$/; @@ -95,91 +58,67 @@ class ExportsCache { } } -function normalizeModules(modules: unknown): string[] { - if (!Array.isArray(modules) || modules.length === 0) { - throw new TypeError( - "Braintrust require-in-the-middle requires a non-empty modules array", - ); - } - - const normalized: string[] = []; - for (const each of modules) { - if (typeof each !== "string") { - throw new TypeError( - "Braintrust require-in-the-middle only supports string module names or absolute paths", - ); - } - normalized.push(each); - } +class Hook { + private readonly _cache = new ExportsCache(); + private readonly _origRequire: HookedRequire; + private readonly _patching = new Set(); + private readonly _normalizedModules: string[]; + private _getBuiltinModule?: GetBuiltinModuleFn; + private _origGetBuiltinModule?: GetBuiltinModuleFn; + private _require: HookedRequire; + private _unhooked = false; + + constructor( + modules: readonly string[], + private readonly _onRequireFn: OnRequireFn, + ) { + this._normalizedModules = Array.from(modules); + this._origRequire = Module.prototype.require; + const self = this; - return normalized; -} + this._require = Module.prototype.require = function ( + this: NodeJS.Module, + id: string, + ) { + if (self._unhooked === true) { + return self._origRequire.call(this, id); + } -const Hook: HookImplementationConstructor = function ( - this: HookInstance | undefined, - modules: unknown, - onrequire: unknown, -): HookInstance | void { - if (!this || !(this instanceof Hook)) { - return new Hook(modules, onrequire); - } + return self._patchedRequire(this, id, false); + }; - const normalizedModules = normalizeModules(modules); - if (typeof onrequire !== "function") { - throw new TypeError( - "Braintrust require-in-the-middle requires an onrequire function", - ); - } - const onRequireFn = onrequire as OnRequireFn; + const processWithGetBuiltinModule: ProcessWithGetBuiltinModule = process; + if (typeof processWithGetBuiltinModule.getBuiltinModule === "function") { + this._origGetBuiltinModule = processWithGetBuiltinModule.getBuiltinModule; + this._getBuiltinModule = processWithGetBuiltinModule.getBuiltinModule = + function (this: unknown, id: string) { + if (self._unhooked === true) { + return self._origGetBuiltinModule!.call(this, id); + } - if (typeof ModuleInternals._resolveFilename !== "function") { - throw new Error( - `Expected Module._resolveFilename to be a function, got ${typeof ModuleInternals._resolveFilename}`, - ); + return self._patchedRequire(this as NodeJS.Module, id, true); + }; + } } - this._cache = new ExportsCache(); - this._unhooked = false; - this._origRequire = Module.prototype.require; + unhook(): void { + this._unhooked = true; - const self = this; - const patching = new Set(); - - this._require = Module.prototype.require = function ( - this: NodeJS.Module, - id: string, - ) { - if (self._unhooked === true) { - return self._origRequire.call(this, id); + if (this._require === Module.prototype.require) { + Module.prototype.require = this._origRequire; } - return patchedRequire.call(this, id, false); - }; - - const processWithGetBuiltinModule: ProcessWithGetBuiltinModule = process; - if (typeof processWithGetBuiltinModule.getBuiltinModule === "function") { - this._origGetBuiltinModule = processWithGetBuiltinModule.getBuiltinModule; - this._getBuiltinModule = processWithGetBuiltinModule.getBuiltinModule = - function (this: unknown, id: string) { - if (self._unhooked === true) { - return getOrigGetBuiltinModule().call(this, id); - } - - return patchedRequire.call(this as NodeJS.Module, id, true); - }; - } - - function getOrigGetBuiltinModule(): GetBuiltinModuleFn { - if (!self._origGetBuiltinModule) { - throw new Error( - "Expected process.getBuiltinModule to be captured before patching builtins", - ); + const processWithGetBuiltinModule: ProcessWithGetBuiltinModule = process; + if ( + this._getBuiltinModule && + this._getBuiltinModule === processWithGetBuiltinModule.getBuiltinModule + ) { + processWithGetBuiltinModule.getBuiltinModule = this._origGetBuiltinModule; } - return self._origGetBuiltinModule; } - function patchedRequire( - this: NodeJS.Module, + private _patchedRequire( + requireThis: NodeJS.Module, id: string, coreOnly: boolean, ): unknown { @@ -194,43 +133,43 @@ const Hook: HookImplementationConstructor = function ( } } } else if (coreOnly) { - return getOrigGetBuiltinModule().call(this, id); + return this._origGetBuiltinModule!.call(requireThis, id); } else { try { - filename = ModuleInternals._resolveFilename(id, this); + filename = ModuleInternals._resolveFilename(id, requireThis); } catch { - return self._origRequire.call(this, id); + return this._origRequire.call(requireThis, id); } } - if (self._cache.has(filename, core) === true) { - return self._cache.get(filename, core); + if (this._cache.has(filename, core) === true) { + return this._cache.get(filename, core); } - const isPatching = patching.has(filename); + const isPatching = this._patching.has(filename); if (isPatching === false) { - patching.add(filename); + this._patching.add(filename); } const exports = coreOnly - ? getOrigGetBuiltinModule().call(this, id) - : self._origRequire.call(this, id); + ? this._origGetBuiltinModule!.call(requireThis, id) + : this._origRequire.call(requireThis, id); if (isPatching === true) { return exports; } - patching.delete(filename); + this._patching.delete(filename); let moduleName: string; let basedir: string | undefined; if (core === true) { - if (normalizedModules.includes(filename) === false) { + if (this._normalizedModules.includes(filename) === false) { return exports; } moduleName = filename; - } else if (normalizedModules.includes(filename)) { + } else if (this._normalizedModules.includes(filename)) { const parsedPath = path.parse(filename); moduleName = parsedPath.name; basedir = parsedPath.dir; @@ -244,20 +183,20 @@ const Hook: HookImplementationConstructor = function ( const fullModuleName = resolveModuleName(stat); let matchFound = false; - if (!id.startsWith(".") && normalizedModules.includes(id)) { + if (!id.startsWith(".") && this._normalizedModules.includes(id)) { moduleName = id; matchFound = true; } if ( - !normalizedModules.includes(moduleName) && - !normalizedModules.includes(fullModuleName) + !this._normalizedModules.includes(moduleName) && + !this._normalizedModules.includes(fullModuleName) ) { return exports; } if ( - normalizedModules.includes(fullModuleName) && + this._normalizedModules.includes(fullModuleName) && fullModuleName !== moduleName ) { moduleName = fullModuleName; @@ -267,46 +206,26 @@ const Hook: HookImplementationConstructor = function ( if (!matchFound) { let res: string; try { - res = require.resolve(moduleName, { paths: [basedir as string] }); + res = require.resolve(moduleName, { paths: [basedir] }); } catch { - self._cache.set(filename, exports, core); + this._cache.set(filename, exports, core); return exports; } if (res !== filename) { - self._cache.set(filename, exports, core); + this._cache.set(filename, exports, core); return exports; } } } - self._cache.set(filename, exports, core); - const patchedExports = onRequireFn(exports, moduleName, basedir); - self._cache.set(filename, patchedExports, core); + this._cache.set(filename, exports, core); + const patchedExports = this._onRequireFn(exports, moduleName, basedir); + this._cache.set(filename, patchedExports, core); return patchedExports; } -} as HookImplementationConstructor; - -Hook.prototype.unhook = function (this: HookInstance): void { - this._unhooked = true; - - if (this._require === Module.prototype.require) { - Module.prototype.require = this._origRequire; - } - - const processWithGetBuiltinModule: ProcessWithGetBuiltinModule = process; - if (processWithGetBuiltinModule.getBuiltinModule !== undefined) { - if ( - this._getBuiltinModule === processWithGetBuiltinModule.getBuiltinModule - ) { - if (this._origGetBuiltinModule) { - processWithGetBuiltinModule.getBuiltinModule = - this._origGetBuiltinModule; - } - } - } -}; +} module.exports = Hook; module.exports.Hook = Hook; diff --git a/js/tests/auto-instrumentations/fixtures/vendor-hooks/iitm-async-app.mjs b/js/tests/auto-instrumentations/fixtures/vendor-hooks/iitm-async-app.mjs index e6d69f304..09dbc6e45 100644 --- a/js/tests/auto-instrumentations/fixtures/vendor-hooks/iitm-async-app.mjs +++ b/js/tests/auto-instrumentations/fixtures/vendor-hooks/iitm-async-app.mjs @@ -2,9 +2,13 @@ import assert from "node:assert"; import getLabel, { foo } from "hook-target"; import getUnhookedLabel, { foo as unhookedFoo } from "unhooked-target"; import cjsTarget from "cjs-hook-target"; +import { nestedValue } from "cjs-reexport-target"; +import { sawIitmParam } from "./query-param-target.mjs?iitm=true"; assert.equal(foo, 57); assert.equal(getLabel(), "patched"); assert.equal(unhookedFoo, 10); assert.equal(getUnhookedLabel(), "untouched"); assert.equal(cjsTarget.value, 8); +assert.equal(nestedValue, "nested"); +assert.equal(sawIitmParam, true); diff --git a/js/tests/auto-instrumentations/fixtures/vendor-hooks/iitm-async-setup.mjs b/js/tests/auto-instrumentations/fixtures/vendor-hooks/iitm-async-setup.mjs index 7d48dcf18..5e2c39ef7 100644 --- a/js/tests/auto-instrumentations/fixtures/vendor-hooks/iitm-async-setup.mjs +++ b/js/tests/auto-instrumentations/fixtures/vendor-hooks/iitm-async-setup.mjs @@ -12,24 +12,27 @@ const hookUrl = new URL( const { addHookMessagePort, registerOptions, waitForAllMessagesAcknowledged } = createAddHookMessageChannel(); -register(hookUrl.href, import.meta.url, registerOptions); +register(hookUrl, import.meta.url, registerOptions); globalThis.__braintrustIitmAsyncHookCalls = 0; -const hook = new Hook(["hook-target", "cjs-hook-target"], (exports, name) => { - globalThis.__braintrustIitmAsyncHookCalls++; - if (name === "hook-target") { - exports.foo += 15; - exports.default = () => "patched"; - } - if (name === "cjs-hook-target") { - exports.default.value = 8; - } -}); +const hook = new Hook( + ["hook-target", "cjs-hook-target", "cjs-reexport-target"], + (exports, name) => { + globalThis.__braintrustIitmAsyncHookCalls++; + if (name === "hook-target") { + exports.foo += 15; + exports.default = () => "patched"; + } + if (name === "cjs-hook-target") { + exports.default.value = 8; + } + }, +); await waitForAllMessagesAcknowledged(); process.on("exit", () => { - assert.equal(globalThis.__braintrustIitmAsyncHookCalls, 2); + assert.equal(globalThis.__braintrustIitmAsyncHookCalls, 3); hook.unhook(); addHookMessagePort.close(); }); diff --git a/js/tests/auto-instrumentations/fixtures/vendor-hooks/iitm-sync-app.mjs b/js/tests/auto-instrumentations/fixtures/vendor-hooks/iitm-sync-app.mjs index 05b1cf2ba..432600f46 100644 --- a/js/tests/auto-instrumentations/fixtures/vendor-hooks/iitm-sync-app.mjs +++ b/js/tests/auto-instrumentations/fixtures/vendor-hooks/iitm-sync-app.mjs @@ -14,7 +14,7 @@ register(); let calls = 0; const hook = new Hook( - ["hook-target", "cjs-hook-target", "fs"], + ["hook-target", "cjs-hook-target", "cjs-reexport-target", "fs"], (exports, name) => { calls++; if (name === "hook-target") { @@ -33,6 +33,8 @@ const hook = new Hook( const target = await import("hook-target"); const other = await import("unhooked-target"); const cjsTarget = await import("cjs-hook-target"); +const cjsReexportTarget = await import("cjs-reexport-target"); +const queryParamTarget = await import("./query-param-target.mjs?iitm=true"); const fs = await import("node:fs"); assert.equal(target.foo, 57); @@ -40,11 +42,14 @@ assert.equal(target.default(), "patched"); assert.equal(other.foo, 10); assert.equal(other.default(), "untouched"); assert.equal(cjsTarget.default.value, 8); +assert.equal(cjsReexportTarget.nestedValue, "nested"); +assert.equal(cjsReexportTarget.rootValue, undefined); +assert.equal(queryParamTarget.sawIitmParam, true); assert.equal(fs.existsSync("/definitely/not/a/real/path"), true); const require = createRequire(import.meta.url); const requiredFs = require("fs"); assert.equal(Object.isExtensible(requiredFs), true); -assert.equal(calls, 3); +assert.equal(calls, 4); hook.unhook(); diff --git a/js/tests/auto-instrumentations/fixtures/vendor-hooks/node_modules/cjs-reexport-leaf/index.cjs b/js/tests/auto-instrumentations/fixtures/vendor-hooks/node_modules/cjs-reexport-leaf/index.cjs new file mode 100644 index 000000000..a8b25ed8f --- /dev/null +++ b/js/tests/auto-instrumentations/fixtures/vendor-hooks/node_modules/cjs-reexport-leaf/index.cjs @@ -0,0 +1 @@ +exports.rootValue = "root"; diff --git a/js/tests/auto-instrumentations/fixtures/vendor-hooks/node_modules/cjs-reexport-leaf/package.json b/js/tests/auto-instrumentations/fixtures/vendor-hooks/node_modules/cjs-reexport-leaf/package.json new file mode 100644 index 000000000..dbeb183ab --- /dev/null +++ b/js/tests/auto-instrumentations/fixtures/vendor-hooks/node_modules/cjs-reexport-leaf/package.json @@ -0,0 +1,5 @@ +{ + "name": "cjs-reexport-leaf", + "version": "1.0.0", + "main": "./index.cjs" +} diff --git a/js/tests/auto-instrumentations/fixtures/vendor-hooks/node_modules/cjs-reexport-target/index.cjs b/js/tests/auto-instrumentations/fixtures/vendor-hooks/node_modules/cjs-reexport-target/index.cjs new file mode 100644 index 000000000..22730cdc1 --- /dev/null +++ b/js/tests/auto-instrumentations/fixtures/vendor-hooks/node_modules/cjs-reexport-target/index.cjs @@ -0,0 +1 @@ +module.exports = require("cjs-reexport-leaf"); diff --git a/js/tests/auto-instrumentations/fixtures/vendor-hooks/node_modules/cjs-reexport-target/node_modules/cjs-reexport-leaf/index.cjs b/js/tests/auto-instrumentations/fixtures/vendor-hooks/node_modules/cjs-reexport-target/node_modules/cjs-reexport-leaf/index.cjs new file mode 100644 index 000000000..673c5564d --- /dev/null +++ b/js/tests/auto-instrumentations/fixtures/vendor-hooks/node_modules/cjs-reexport-target/node_modules/cjs-reexport-leaf/index.cjs @@ -0,0 +1 @@ +exports.nestedValue = "nested"; diff --git a/js/tests/auto-instrumentations/fixtures/vendor-hooks/node_modules/cjs-reexport-target/node_modules/cjs-reexport-leaf/package.json b/js/tests/auto-instrumentations/fixtures/vendor-hooks/node_modules/cjs-reexport-target/node_modules/cjs-reexport-leaf/package.json new file mode 100644 index 000000000..dbeb183ab --- /dev/null +++ b/js/tests/auto-instrumentations/fixtures/vendor-hooks/node_modules/cjs-reexport-target/node_modules/cjs-reexport-leaf/package.json @@ -0,0 +1,5 @@ +{ + "name": "cjs-reexport-leaf", + "version": "1.0.0", + "main": "./index.cjs" +} diff --git a/js/tests/auto-instrumentations/fixtures/vendor-hooks/node_modules/cjs-reexport-target/package.json b/js/tests/auto-instrumentations/fixtures/vendor-hooks/node_modules/cjs-reexport-target/package.json new file mode 100644 index 000000000..3e6393aca --- /dev/null +++ b/js/tests/auto-instrumentations/fixtures/vendor-hooks/node_modules/cjs-reexport-target/package.json @@ -0,0 +1,8 @@ +{ + "name": "cjs-reexport-target", + "version": "1.0.0", + "main": "./index.cjs", + "exports": { + ".": "./index.cjs" + } +} diff --git a/js/tests/auto-instrumentations/fixtures/vendor-hooks/query-param-target.mjs b/js/tests/auto-instrumentations/fixtures/vendor-hooks/query-param-target.mjs new file mode 100644 index 000000000..254f246cf --- /dev/null +++ b/js/tests/auto-instrumentations/fixtures/vendor-hooks/query-param-target.mjs @@ -0,0 +1 @@ +export const sawIitmParam = import.meta.url.includes("iitm=true"); diff --git a/js/tests/auto-instrumentations/fixtures/vendor-hooks/ritm-coexist-app.cjs b/js/tests/auto-instrumentations/fixtures/vendor-hooks/ritm-coexist-app.cjs new file mode 100644 index 000000000..2c30a0d36 --- /dev/null +++ b/js/tests/auto-instrumentations/fixtures/vendor-hooks/ritm-coexist-app.cjs @@ -0,0 +1,52 @@ +const assert = require("node:assert"); +const Module = require("node:module"); +const { + Hook, +} = require("../../../../src/auto-instrumentations/require-in-the-middle/index.ts"); + +const originalRequire = Module.prototype.require; + +function clearTarget() { + delete require.cache[require.resolve("ritm-target")]; +} + +function wrapRequire(previousRequire) { + return function foreignRequire(id) { + const exports = previousRequire.call(this, id); + if (id === "ritm-target") { + return { ...exports, value: exports.value + 100 }; + } + return exports; + }; +} + +try { + Module.prototype.require = wrapRequire(originalRequire); + const hookAfterForeign = new Hook(["ritm-target"], (exports) => { + return { ...exports, value: exports.value + 10 }; + }); + + assert.equal(require("ritm-target").value, 111); + hookAfterForeign.unhook(); + clearTarget(); + assert.equal(require("ritm-target").value, 101); +} finally { + Module.prototype.require = originalRequire; + clearTarget(); +} + +try { + const hookBeforeForeign = new Hook(["ritm-target"], (exports) => { + return { ...exports, value: exports.value + 10 }; + }); + Module.prototype.require = wrapRequire(Module.prototype.require); + + clearTarget(); + assert.equal(require("ritm-target").value, 111); + hookBeforeForeign.unhook(); + clearTarget(); + assert.equal(require("ritm-target").value, 101); +} finally { + Module.prototype.require = originalRequire; + clearTarget(); +} diff --git a/js/tests/auto-instrumentations/import-require-in-the-middle.test.ts b/js/tests/auto-instrumentations/import-require-in-the-middle.test.ts index 5727db474..4794d3b22 100644 --- a/js/tests/auto-instrumentations/import-require-in-the-middle.test.ts +++ b/js/tests/auto-instrumentations/import-require-in-the-middle.test.ts @@ -31,6 +31,13 @@ describe("vendored import-in-the-middle and require-in-the-middle", () => { cwd: fixturesDir, }); }); + + it("coexists with other CommonJS require wrappers", async () => { + await runNode({ + args: [path.join(fixturesDir, "ritm-coexist-app.cjs")], + cwd: fixturesDir, + }); + }); }); function runNode({