Skip to content
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions src/codegen/class-definitions.ts
Original file line number Diff line number Diff line change
Expand Up @@ -152,6 +152,11 @@ export class ClassDefinition {
* The instances of this class are intended to be inside the this of a bound function.
*/
forBind?: boolean;
/**
* Parent of the generated prototype object. "Error" puts Error.prototype in
* the chain so instances satisfy `instanceof Error`. Default: Object.prototype.
*/
prototypeBase?: "Error";
/**
* ## IMPORTANT
* You _must_ free the pointer to your native class!
Expand Down
8 changes: 7 additions & 1 deletion src/codegen/generate-classes.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1837,7 +1837,13 @@ ${

JSObject* ${name}::createPrototype(VM& vm, JSDOMGlobalObject* globalObject)
{
auto *structure = ${prototypeName(typeName)}::createStructure(vm, globalObject, ${obj.forBind ? "globalObject->functionPrototype()" : "globalObject->objectPrototype()"});
auto *structure = ${prototypeName(typeName)}::createStructure(vm, globalObject, ${
obj.forBind
? "globalObject->functionPrototype()"
: obj.prototypeBase === "Error"
? "globalObject->errorPrototype()"
: "globalObject->objectPrototype()"
});
structure->setMayBePrototype(true);
return ${prototypeName(typeName)}::create(vm, globalObject, structure);
}
Expand Down
11 changes: 11 additions & 0 deletions src/js/builtins/CommonJS.ts
Original file line number Diff line number Diff line change
Expand Up @@ -149,6 +149,17 @@
id: string,
options: { paths?: string[] } = {},
) {
if (typeof options === "object" && options !== null && options.paths !== undefined) {

Check failure on line 152 in src/js/builtins/CommonJS.ts

View workflow job for this annotation

GitHub Actions / Lint JavaScript

bun(no-duplicate-conditional-property-access)

`options.paths` is read in the `if` condition and again in the body. Read it into a local first (e.g. `const { paths } = options`) so the property is only accessed once.
const paths = options.paths;
if (!$isArray(paths)) {
throw $ERR_INVALID_ARG_VALUE("options.paths", paths);
}
for (let i = 0; i < paths.length; i++) {
if (typeof paths[i] !== "string") {
throw $ERR_INVALID_ARG_TYPE("paths", "array of strings", paths);
}
}
}
return $resolveSync(
id,
typeof this === "string" ? this : (this?.filename ?? this?.id ?? ""),
Expand Down
125 changes: 125 additions & 0 deletions src/jsc/ResolveMessage.rs
Original file line number Diff line number Diff line change
Expand Up @@ -56,6 +56,22 @@ fn import_kind_label(kind: ImportKind) -> &'static [u8] {
}
}

/// First path segment of a bare specifier ("@scope/name" keeps two),
/// matching Node's ERR_MODULE_NOT_FOUND "Cannot find package '<name>'".
fn esm_package_name(specifier: &[u8]) -> &[u8] {
let slash_after = |from: usize| {
specifier[from..]
.iter()
.position(|&b| b == b'/')
.map_or(specifier.len(), |i| from + i)
};
let mut end = slash_after(0);
if specifier.starts_with(b"@") && end < specifier.len() {
end = slash_after(end + 1);
}
&specifier[..end]
}

impl ResolveMessage {
// `#[JsClass]` emits `ResolveMessageClass__construct` calling this.
pub fn constructor(
Expand Down Expand Up @@ -325,11 +341,120 @@ impl ResolveMessage {
))
}

/// Module-not-found for a runtime import kind whose `.message` /
/// `.requireStack` should match Node.js. Returns `(import_kind, specifier,
/// usable_referrer)`; `None` keeps the original Bun-formatted text.
fn node_error_shape(&self) -> Option<(ImportKind, &[u8], Option<&[u8]>)> {
let bun_ast::Metadata::Resolve(resolve) = &self.msg.metadata else {
return None;
};
match resolve.import_kind {
ImportKind::Require
| ImportKind::RequireResolve
| ImportKind::Stmt
| ImportKind::Dynamic => {}
_ => return None,
}
// Fallback paths tag every CrateError as `ModuleNotFound`, so gate on
// the formatted text rather than `resolve.err` to leave InvalidURL /
// InvalidDataURL / ENAMETOOLONG messages untouched.
let text: &[u8] = &self.msg.data.text;
if !(text.starts_with(b"Cannot find module '")
|| text.starts_with(b"Cannot find package '"))
{
return None;
}
// `require.resolve('node:missing')` is a plain MODULE_NOT_FOUND in
// Node; every other kind reports ERR_UNKNOWN_BUILTIN_MODULE instead.
let specifier = resolve.specifier.slice(&self.msg.data.text);
if specifier.starts_with(b"node:") && resolve.import_kind != ImportKind::RequireResolve {
return None;
}
let referrer = self
.referrer
.as_deref()
.filter(|r| !r.is_empty() && *r != b"bun:main");
Some((resolve.import_kind, specifier, referrer))
}

/// Node's message for a module-not-found error, or `None` when the
/// original text should be kept.
fn node_message(&self) -> Option<Vec<u8>> {
use bstr::BStr;
let (kind, specifier, referrer) = self.node_error_shape()?;
let mut out = Vec::new();
match kind {
ImportKind::Require | ImportKind::RequireResolve => {
write!(&mut out, "Cannot find module '{}'", BStr::new(specifier)).ok();
if let Some(referrer) = referrer {
write!(&mut out, "\nRequire stack:\n- {}", BStr::new(referrer)).ok();
}
}
ImportKind::Stmt | ImportKind::Dynamic => {
let referrer = referrer?;
if bun_resolver::is_package_path(specifier) {
write!(
&mut out,
"Cannot find package '{}' imported from {}",
BStr::new(esm_package_name(specifier)),
BStr::new(referrer),
)
.ok();
} else {
write!(
&mut out,
"Cannot find module '{}' imported from {}",
BStr::new(specifier),
BStr::new(referrer),
)
.ok();
}
}
_ => return None,
}
Some(out)
}

#[crate::host_fn(getter)]
pub fn get_message(this: &Self, global: &JSGlobalObject) -> JsResult<JSValue> {
if let Some(text) = this.node_message() {
return Ok(ZigString::init_utf8(&text).to_js(global));
}
Ok(ZigString::init_utf8(&this.msg.data.text).to_js(global))
}

// Node: MODULE_NOT_FOUND errors carry `requireStack` (the chain of
// requiring files; Bun tracks only the direct referrer). CJS kinds only.
#[crate::host_fn(getter)]
pub fn get_require_stack(this: &Self, global: &JSGlobalObject) -> JsResult<JSValue> {
let Some((kind, _, referrer)) = this.node_error_shape() else {
return Ok(JSValue::UNDEFINED);
};
if !matches!(kind, ImportKind::Require | ImportKind::RequireResolve) {
return Ok(JSValue::UNDEFINED);
}
let mut entries: Vec<&[u8]> = Vec::new();
if let Some(r) = referrer {
entries.push(r);
}
JSValue::create_array_from_iter(global, entries.iter().copied(), |r| {
Ok(ZigString::init_utf8(r).to_js(global))
})
}

// A synthesized `name: message` header; Bun does not capture JS frames at
// module-resolution time, so there are no `at ...` lines.
#[crate::host_fn(getter)]
pub fn get_stack(this: &Self, global: &JSGlobalObject) -> JsResult<JSValue> {
let mut out = Vec::new();
out.extend_from_slice(b"ResolveMessage: ");
match this.node_message() {
Some(text) => out.extend_from_slice(&text),
None => out.extend_from_slice(&this.msg.data.text),
}
Ok(ZigString::init_utf8(&out).to_js(global))
}

#[crate::host_fn(getter)]
pub fn get_level(this: &Self, global: &JSGlobalObject) -> JsResult<JSValue> {
Ok(ZigString::init(this.msg.kind.string()).to_js(global))
Expand Down
11 changes: 11 additions & 0 deletions src/jsc/resolve_message.classes.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,8 @@ export default [
construct: true,
finalize: true,
configurable: false,
// Error.prototype in the chain: userland checks `err instanceof Error`.
prototypeBase: "Error",
klass: {},
JSType: "0b11101110",
proto: {
Expand All @@ -18,6 +20,15 @@ export default [
getter: "getCode",
cache: true,
},
requireStack: {
getter: "getRequireStack",
cache: true,
},
stack: {
getter: "getStack",
cache: true,
writable: true,
},
name: {
value: "ResolveMessage",
},
Expand Down
2 changes: 1 addition & 1 deletion test/js/bun/resolve/import-meta.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -73,7 +73,7 @@ it("Module.createRequire does not use file url as the referrer (err message chec
expect(e.name).not.toBe("UnreachableError");
expect(e.message).not.toInclude("file:///");
expect(e.message).toInclude(`'whaaat'`);
expect(e.message).toInclude(`'` + import.meta.path + `'`);
expect(e.message).toInclude(import.meta.path);
}
});

Expand Down
2 changes: 1 addition & 1 deletion test/js/bun/resolve/resolve-error.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -117,7 +117,7 @@ describe("ResolveMessage", () => {
expect(async () => {
// @ts-ignore
await import(":://filesystem");
}).toThrow("Cannot find module");
}).toThrow("Cannot find package '::'");
});

it("referrer is not freed before it is read", () => {
Expand Down
14 changes: 7 additions & 7 deletions test/js/node/missing-module.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ test("not implemented yet module throws an error", () => {
code: "ERR_UNKNOWN_BUILTIN_MODULE",
});
assert.throws(() => require.resolve(missingModule), {
message: /Cannot find package 'node:missing' from/,
message: /^Cannot find module 'node:missing'\nRequire stack:\n- /,
code: "MODULE_NOT_FOUND",
});
assert.rejects(() => import(missingModule), {
Expand All @@ -21,15 +21,15 @@ test("not implemented yet module throws an error", () => {
});

assert.throws(() => require(missingBun), {
message: /^Cannot find package 'bun:missing' from/,
message: /^Cannot find module 'bun:missing'\nRequire stack:\n- /,
code: "MODULE_NOT_FOUND",
});
assert.throws(() => require.resolve(missingBun), {
message: /^Cannot find package 'bun:missing' from/,
message: /^Cannot find module 'bun:missing'\nRequire stack:\n- /,
code: "MODULE_NOT_FOUND",
});
assert.rejects(() => import(missingBun), {
message: /^Cannot find package 'bun:missing' from/,
message: /^Cannot find package 'bun:missing' imported from /,
code: "ERR_MODULE_NOT_FOUND",
});

Expand All @@ -47,15 +47,15 @@ test("not implemented yet module throws an error", () => {
});

assert.throws(() => require(missingPackage), {
message: /^Cannot find package 'package-that-doesnt-exist'/,
message: /^Cannot find module 'package-that-doesnt-exist'\nRequire stack:\n- /,
code: "MODULE_NOT_FOUND",
});
assert.throws(() => require.resolve(missingPackage), {
message: /^Cannot find package 'package-that-doesnt-exist'/,
message: /^Cannot find module 'package-that-doesnt-exist'\nRequire stack:\n- /,
code: "MODULE_NOT_FOUND",
});
assert.rejects(() => import(missingPackage), {
message: /^Cannot find package 'package-that-doesnt-exist'/,
message: /^Cannot find package 'package-that-doesnt-exist' imported from /,
code: "ERR_MODULE_NOT_FOUND",
});
});
18 changes: 18 additions & 0 deletions test/js/node/test/parallel/test-require-resolve-invalid-paths.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,18 @@
'use strict';

require('../common');
const assert = require('assert');

// Test invalid `paths` entries: Ensure non-string entries throw an error
{
const paths = [1, false, null, undefined, () => {}, {}];
paths.forEach((value) => {
assert.throws(
() => require.resolve('.', { paths: [value] }),
{
name: 'TypeError',
code: 'ERR_INVALID_ARG_TYPE',
}
);
});
}
Loading