diff --git a/src/jsc/JSPromise.rs b/src/jsc/JSPromise.rs index 98ed5f3ade03..a30db57413c8 100644 --- a/src/jsc/JSPromise.rs +++ b/src/jsc/JSPromise.rs @@ -266,10 +266,8 @@ impl JSPromise { return value; } - if value.is_any_error() { - return Self::dangerously_create_rejected_promise_value_without_notifying_vm( - global, value, - ); + if let Some(err) = value.to_error() { + return Self::rejected_promise(global, err).to_js(); } Self::resolved_promise_value(global, value) @@ -327,6 +325,18 @@ impl JSPromise { JSPromise::opaque_mut(JSC__JSPromise__rejectedPromise(global, value)) } + /// Create a new promise rejected with the exception `err` proves is pending, + /// taking it off the VM. The reason is converted like [`reject`](Self::reject) + /// does; a termination is propagated instead of becoming a reason. + pub fn rejected_promise_with_caught_exception( + global: &JSGlobalObject, + err: JsError, + ) -> Result<&mut JSPromise, JsTerminated> { + let promise = Self::create(global); + promise.reject(global, Err(err))?; + Ok(promise) + } + /// **DEPRECATED** use `rejected_promise` instead. /// /// Create a new rejected promise without notifying the VM. Unhandled diff --git a/src/jsc/bindings/ImportMetaObject.h b/src/jsc/bindings/ImportMetaObject.h index 761959f17bb5..b3bc62f69d8f 100644 --- a/src/jsc/bindings/ImportMetaObject.h +++ b/src/jsc/bindings/ImportMetaObject.h @@ -10,7 +10,6 @@ extern "C" JSC_DECLARE_HOST_FUNCTION(functionImportMeta__resolveSync); extern "C" JSC_DECLARE_HOST_FUNCTION(functionImportMeta__resolveSyncPrivate); -extern "C" JSC::EncodedJSValue Bun__resolve(JSC::JSGlobalObject* global, JSC::EncodedJSValue specifier, JSC::EncodedJSValue from, bool is_esm); extern "C" JSC::EncodedJSValue Bun__resolveSync(JSC::JSGlobalObject* global, JSC::EncodedJSValue specifier, JSC::EncodedJSValue from, bool is_esm, bool isUserRequireResolve); extern "C" JSC::EncodedJSValue Bun__resolveSyncWithPaths(JSC::JSGlobalObject* global, JSC::EncodedJSValue specifier, JSC::EncodedJSValue from, bool is_esm, bool isUserRequireResolve, const BunString* paths, size_t paths_len); extern "C" JSC::EncodedJSValue Bun__resolveSyncWithSource(JSC::JSGlobalObject* global, JSC::EncodedJSValue specifier, BunString* from, bool is_esm, bool isUserRequireResolve); diff --git a/src/runtime/api/BunObject.rs b/src/runtime/api/BunObject.rs index fdedb2be588f..259fa333e8c7 100644 --- a/src/runtime/api/BunObject.rs +++ b/src/runtime/api/BunObject.rs @@ -1219,53 +1219,14 @@ fn resolve(global_object: &JSGlobalObject, callframe: &CallFrame) -> JsResult v, Err(e) => { - let err = global_object.take_error(e); return Ok( - JSPromise::dangerously_create_rejected_promise_value_without_notifying_vm( - global_object, - err, - ), + JSPromise::rejected_promise_with_caught_exception(global_object, e)?.to_js(), ); } }; Ok(JSPromise::resolved_promise_value(global_object, value)) } -// HOST_EXPORT(Bun__resolve, c) -pub fn bun_resolve( - global: &JSGlobalObject, - specifier: JSValue, - source: JSValue, - is_esm: bool, -) -> JSValue { - let Ok(specifier_str) = specifier.to_bun_string(global) else { - return JSValue::ZERO; - }; - let specifier_str = scopeguard::guard(specifier_str, |s| s.deref()); - - let Ok(source_str) = source.to_bun_string(global) else { - return JSValue::ZERO; - }; - let source_str = scopeguard::guard(source_str, |s| s.deref()); - - let value = match do_resolve_with_args::( - global, - *specifier_str, - *source_str, - ResolveMode::from_ffi_bools(is_esm, false), - ) { - Ok(v) => v, - Err(_) => { - let err = global.try_take_exception().unwrap(); - return JSPromise::dangerously_create_rejected_promise_value_without_notifying_vm( - global, err, - ); - } - }; - - JSPromise::resolved_promise_value(global, value) -} - // HOST_EXPORT(Bun__resolveSync, c) pub fn bun_resolve_sync( global: &JSGlobalObject, diff --git a/src/runtime/api/bun/subprocess.rs b/src/runtime/api/bun/subprocess.rs index e8797b6454b1..838ba7359836 100644 --- a/src/runtime/api/bun/subprocess.rs +++ b/src/runtime/api/bun/subprocess.rs @@ -1384,10 +1384,7 @@ impl Subprocess<'_> { ), Status::Err(err) => { let js_err = err.to_js(global_this); - JSPromise::dangerously_create_rejected_promise_value_without_notifying_vm( - global_this, - js_err, - ) + JSPromise::rejected_promise(global_this, js_err).to_js() } _ => { let promise = JSPromise::create(global_this).to_js(); diff --git a/src/runtime/node/node_fs_binding.rs b/src/runtime/node/node_fs_binding.rs index 99415ff3c126..6eaa13ce66f9 100644 --- a/src/runtime/node/node_fs_binding.rs +++ b/src/runtime/node/node_fs_binding.rs @@ -103,11 +103,7 @@ fn run_async( if A::HAVE_ABORT_SIGNAL { if let Some(signal) = args.signal() { if let Some(abort_error) = signal.node_abort_error_if_aborted(global) { - let promise = - JSPromise::dangerously_create_rejected_promise_value_without_notifying_vm( - global, - abort_error, - ); + let promise = JSPromise::rejected_promise(global, abort_error).to_js(); args.unprotect(); drop(args); // SAFETY: not yet dropped; only drop site for this path. diff --git a/src/runtime/server/server_body.rs b/src/runtime/server/server_body.rs index 200ea2acbeee..f0b5eb6f0ee6 100644 --- a/src/runtime/server/server_body.rs +++ b/src/runtime/server/server_body.rs @@ -2426,24 +2426,22 @@ where jsc::mark_binding!(); if self.config.on_request.is_empty() { - return Ok( - JSPromise::dangerously_create_rejected_promise_value_without_notifying_vm( - ctx, - ZigString::init(b"fetch() requires the server to have a fetch handler") - .to_error_instance(ctx), - ), - ); + return Ok(JSPromise::rejected_promise( + ctx, + ZigString::init(b"fetch() requires the server to have a fetch handler") + .to_error_instance(ctx), + ) + .to_js()); } let arguments = callframe.arguments(); if arguments.is_empty() { let fetch_error = Fetch::FETCH_ERROR_NO_ARGS; - return Ok( - JSPromise::dangerously_create_rejected_promise_value_without_notifying_vm( - ctx, - ZigString::init(fetch_error.as_bytes()).to_error_instance(ctx), - ), - ); + return Ok(JSPromise::rejected_promise( + ctx, + ZigString::init(fetch_error.as_bytes()).to_error_instance(ctx), + ) + .to_js()); } let mut headers: Option = None; @@ -2462,12 +2460,11 @@ where if temp_url_str.is_empty() { let fetch_error = Fetch::FETCH_ERROR_BLANK_URL; - return Ok( - JSPromise::dangerously_create_rejected_promise_value_without_notifying_vm( - ctx, - ZigString::init(fetch_error.as_bytes()).to_error_instance(ctx), - ), - ); + return Ok(JSPromise::rejected_promise( + ctx, + ZigString::init(fetch_error.as_bytes()).to_error_instance(ctx), + ) + .to_js()); } let mut url = URL::parse(temp_url_str); @@ -2516,11 +2513,11 @@ where if let Some(body__) = opts.fast_get(ctx, jsc::BuiltinName::Body)? { match Blob::get::(ctx, body__) { Ok(new_blob) => body = BodyValue::Blob(new_blob), - Err(_) => { - return Ok(JSPromise::dangerously_create_rejected_promise_value_without_notifying_vm( - ctx, - ZigString::init(b"fetch() received invalid body").to_error_instance(ctx), - )); + Err(err) => { + return Ok(JSPromise::rejected_promise_with_caught_exception( + ctx, err, + )? + .to_js()); } } } @@ -2545,9 +2542,7 @@ where } else { let fetch_error = Fetch::fetch_type_error_string(first_arg); let err = jsc::ErrorCode::INVALID_ARG_TYPE.fmt(ctx, format_args!("{}", fetch_error)); - return Ok( - JSPromise::dangerously_create_rejected_promise_value_without_notifying_vm(ctx, err), - ); + return Ok(JSPromise::rejected_promise(ctx, err).to_js()); }; // `Request::to_js` stores `self as *mut @@ -2565,25 +2560,21 @@ where let response_value = match on_request.call(&global_this, self.js_value_assert_alive(), &[request_value]) { Ok(v) => v, - Err(err) => global_this.take_exception(err), + Err(err) => { + return Ok(JSPromise::rejected_promise_with_caught_exception(ctx, err)?.to_js()); + } }; - if response_value.is_any_error() { - return Ok( - JSPromise::dangerously_create_rejected_promise_value_without_notifying_vm( - ctx, - response_value, - ), - ); + if let Some(err) = response_value.to_error() { + return Ok(JSPromise::rejected_promise(ctx, err).to_js()); } if response_value.is_empty_or_undefined_or_null() { - return Ok( - JSPromise::dangerously_create_rejected_promise_value_without_notifying_vm( - ctx, - ZigString::init(b"fetch() returned an empty value").to_error_instance(ctx), - ), - ); + return Ok(JSPromise::rejected_promise( + ctx, + ZigString::init(b"fetch() returned an empty value").to_error_instance(ctx), + ) + .to_js()); } if response_value.as_any_promise().is_some() { diff --git a/src/runtime/webcore/Blob.rs b/src/runtime/webcore/Blob.rs index 3e9b5efb447a..da3d99ddfc00 100644 --- a/src/runtime/webcore/Blob.rs +++ b/src/runtime/webcore/Blob.rs @@ -1393,12 +1393,11 @@ impl BlobExt for Blob { extra_options: Option, ) -> JsResult { let Some(store) = self.store.get().clone() else { - return Ok( - JSPromise::dangerously_create_rejected_promise_value_without_notifying_vm( - global_this, - global_this.create_error_instance(format_args!("Blob is detached")), - ), - ); + return Ok(JSPromise::rejected_promise( + global_this, + global_this.create_error_instance(format_args!("Blob is detached")), + ) + .to_js()); }; if self.is_s3() { @@ -1408,12 +1407,11 @@ impl BlobExt for Blob { let aws_options = match s3.get_credentials_with_options(extra_options, global_this) { Ok(o) => o, Err(err) => { - return Ok( - JSPromise::dangerously_create_rejected_promise_value_without_notifying_vm( - global_this, - global_this.take_exception(err), - ), - ); + return Ok(JSPromise::rejected_promise_with_caught_exception( + global_this, + err, + )? + .to_js()); } }; @@ -1454,12 +1452,11 @@ impl BlobExt for Blob { } if !matches!(store.data, store::Data::File(_)) { - return Ok( - JSPromise::dangerously_create_rejected_promise_value_without_notifying_vm( - global_this, - global_this.create_error_instance(format_args!("Blob is read-only")), - ), - ); + return Ok(JSPromise::rejected_promise( + global_this, + global_this.create_error_instance(format_args!("Blob is read-only")), + ) + .to_js()); } let file_sink: *mut webcore::FileSink = 'brk_sink: { @@ -1478,10 +1475,11 @@ impl BlobExt for Blob { ) { bun_sys::Result::Ok(result) => result, bun_sys::Result::Err(err) => { - return Ok(JSPromise::dangerously_create_rejected_promise_value_without_notifying_vm( + return Ok(JSPromise::rejected_promise( global_this, err.with_path(path).to_js(global_this), - )); + ) + .to_js()); } } }; @@ -1535,10 +1533,11 @@ impl BlobExt for Blob { unsafe { (*sink).writer.with_mut(|w| w.start_sync(fd, false)) } { unsafe { webcore::FileSink::deref(sink) }; - return Ok(JSPromise::dangerously_create_rejected_promise_value_without_notifying_vm( + return Ok(JSPromise::rejected_promise( global_this, err.to_js(global_this), - )); + ) + .to_js()); } } else { // SAFETY: sink is live; sole owner here. @@ -1546,10 +1545,11 @@ impl BlobExt for Blob { unsafe { (*sink).writer.with_mut(|w| w.start(fd, true)) } { unsafe { webcore::FileSink::deref(sink) }; - return Ok(JSPromise::dangerously_create_rejected_promise_value_without_notifying_vm( + return Ok(JSPromise::rejected_promise( global_this, err.to_js(global_this), - )); + ) + .to_js()); } } @@ -1589,10 +1589,7 @@ impl BlobExt for Blob { // SAFETY: release the +1 strong ref taken by `init` on the error path. unsafe { webcore::FileSink::deref(sink) }; return Ok( - JSPromise::dangerously_create_rejected_promise_value_without_notifying_vm( - global_this, - err.to_js(global_this), - ), + JSPromise::rejected_promise(global_this, err.to_js(global_this)).to_js(), ); } break 'brk_sink sink; @@ -1613,12 +1610,7 @@ impl BlobExt for Blob { if let Some(err) = assignment_result.to_error() { // SAFETY: release our +1 ref on the sink. unsafe { webcore::FileSink::deref(file_sink) }; - return Ok( - JSPromise::dangerously_create_rejected_promise_value_without_notifying_vm( - global_this, - err, - ), - ); + return Ok(JSPromise::rejected_promise(global_this, err).to_js()); } if !assignment_result.is_empty_or_undefined_or_null() { @@ -1627,8 +1619,9 @@ impl BlobExt for Blob { assignment_result.ensure_still_alive(); // it returns a Promise when it goes through ReadableStreamDefaultReader if let Some(promise) = assignment_result.as_any_promise() { - match promise.status() { - jsc::js_promise::Status::Pending => { + // `MarkHandled`: a rejection is forwarded to the promise returned below. + match promise.unwrap(global_this.vm(), jsc::PromiseUnwrapMode::MarkHandled) { + jsc::PromiseResult::Pending => { let wrapper = bun_core::heap::into_raw(Box::new(FileStreamWrapper { promise: jsc::JSPromiseStrong::init(global_this), readable_stream_ref: @@ -1648,7 +1641,7 @@ impl BlobExt for Blob { ); return Ok(promise_value); } - jsc::js_promise::Status::Fulfilled => { + jsc::PromiseResult::Fulfilled(_) => { // SAFETY: release our +1 ref on the sink. unsafe { webcore::FileSink::deref(file_sink) }; readable_stream.done(global_this); @@ -1657,26 +1650,18 @@ impl BlobExt for Blob { JSValue::js_number(0.0), )); } - jsc::js_promise::Status::Rejected => { + jsc::PromiseResult::Rejected(err) => { // SAFETY: release our +1 ref on the sink. unsafe { webcore::FileSink::deref(file_sink) }; readable_stream.cancel(global_this); - return Ok(JSPromise::dangerously_create_rejected_promise_value_without_notifying_vm( - global_this, - promise.result(global_this.vm()), - )); + return Ok(JSPromise::rejected_promise(global_this, err).to_js()); } } } else { // SAFETY: release our +1 ref on the sink. unsafe { webcore::FileSink::deref(file_sink) }; readable_stream.cancel(global_this); - return Ok( - JSPromise::dangerously_create_rejected_promise_value_without_notifying_vm( - global_this, - assignment_result, - ), - ); + return Ok(JSPromise::rejected_promise(global_this, assignment_result).to_js()); } } // SAFETY: release our +1 ref on the sink. @@ -4580,12 +4565,7 @@ fn write_file_with_empty_source_to_destination( } *err = sys_error_with_path_like(err, &file.pathlike); - return Ok( - JSPromise::dangerously_create_rejected_promise_value_without_notifying_vm( - ctx, - err.to_js(ctx), - ), - ); + return Ok(JSPromise::rejected_promise(ctx, err.to_js(ctx)).to_js()); } } store::Data::S3(s3) => { @@ -4593,12 +4573,7 @@ fn write_file_with_empty_source_to_destination( let aws_options = match s3.get_credentials_with_options(options.extra_options, ctx) { Ok(o) => o, Err(err) => { - return Ok( - JSPromise::dangerously_create_rejected_promise_value_without_notifying_vm( - ctx, - ctx.take_exception(err), - ), - ); + return Ok(JSPromise::rejected_promise_with_caught_exception(ctx, err)?.to_js()); } }; @@ -4788,14 +4763,11 @@ pub(crate) fn write_file_with_source_destination( options.extra_options, ); } else { - return Ok( - JSPromise::dangerously_create_rejected_promise_value_without_notifying_vm( - ctx, - ctx.create_error_instance(format_args!( - "Failed to stream bytes from s3 bucket" - )), - ), - ); + return Ok(JSPromise::rejected_promise( + ctx, + ctx.create_error_instance(format_args!("Failed to stream bytes from s3 bucket")), + ) + .to_js()); } } else if destination_type == store::DataTag::Bytes && source_type == store::DataTag::Bytes { // If this is bytes <> bytes, we can just duplicate it @@ -4818,12 +4790,7 @@ pub(crate) fn write_file_with_source_destination( let aws_options = match s3.get_credentials_with_options(options.extra_options, ctx) { Ok(o) => o, Err(err) => { - return Ok( - JSPromise::dangerously_create_rejected_promise_value_without_notifying_vm( - ctx, - ctx.take_exception(err), - ), - ); + return Ok(JSPromise::rejected_promise_with_caught_exception(ctx, err)?.to_js()); } }; let proxy_owned = http_proxy_href(ctx); @@ -4862,10 +4829,13 @@ pub(crate) fn write_file_with_source_destination( core::ptr::null_mut(), ); } else { - return Ok(JSPromise::dangerously_create_rejected_promise_value_without_notifying_vm( + return Ok(JSPromise::rejected_promise( ctx, - ctx.create_error_instance(format_args!("Failed to stream bytes to s3 bucket")), - )); + ctx.create_error_instance(format_args!( + "Failed to stream bytes to s3 bucket" + )), + ) + .to_js()); } } else { struct Wrapper { @@ -4962,14 +4932,13 @@ pub(crate) fn write_file_with_source_destination( core::ptr::null_mut(), ); } else { - return Ok( - JSPromise::dangerously_create_rejected_promise_value_without_notifying_vm( - ctx, - ctx.create_error_instance(format_args!( - "Failed to stream bytes to s3 bucket" - )), - ), - ); + return Ok(JSPromise::rejected_promise( + ctx, + ctx.create_error_instance(format_args!( + "Failed to stream bytes to s3 bucket" + )), + ) + .to_js()); } } } @@ -5137,133 +5106,131 @@ pub(crate) fn write_file_internal( // `Response` and `Request` both expose `get_body_value()` / // `get_body_readable_stream()`; one helper takes the // body-value pointer and a `get_stream` closure. - let mut body_dispatch = - |body_value: *mut webcore::body::Value, - get_stream: &mut dyn FnMut(&JSGlobalObject) -> Option| - -> JsResult> { - use core::ops::ControlFlow; - use webcore::body::Value as BodyValue; - enum BodyTag { - Use, - Error, - Locked, + let mut body_dispatch = |body_value: *mut webcore::body::Value, + get_stream: &mut dyn FnMut( + &JSGlobalObject, + ) -> Option| + -> JsResult> { + use core::ops::ControlFlow; + use webcore::body::Value as BodyValue; + enum BodyTag { + Use, + Error, + Locked, + } + // `body_value` is `&mut Body::Value` from a live JS heap + // Response/Request `m_ctx`, held raw so every borrow below is + // scoped and none spans the JS-running calls in the arms. + // SAFETY: scoped shared read of the variant tag. + let tag = match unsafe { &*body_value } { + BodyValue::Error(_) => BodyTag::Error, + BodyValue::Locked(_) => BodyTag::Locked, + _ => BodyTag::Use, + }; + match tag { + BodyTag::Use => { + // SAFETY: exclusive borrow scoped to the call; `use_()` runs no JS. + Ok(ControlFlow::Continue(unsafe { (*body_value).use_() })) } - // `body_value` is `&mut Body::Value` from a live JS heap - // Response/Request `m_ctx`, held raw so every borrow below is - // scoped and none spans the JS-running calls in the arms. - // SAFETY: scoped shared read of the variant tag. - let tag = match unsafe { &*body_value } { - BodyValue::Error(_) => BodyTag::Error, - BodyValue::Locked(_) => BodyTag::Locked, - _ => BodyTag::Use, - }; - match tag { - BodyTag::Use => { - // SAFETY: exclusive borrow scoped to the call; `use_()` runs no JS. - Ok(ControlFlow::Continue(unsafe { (*body_value).use_() })) - } - BodyTag::Error => { - let err_js = { - // SAFETY: exclusive borrow; ends before `use_()` below. - let BodyValue::Error(err_ref) = (unsafe { &mut *body_value }) else { - unreachable!() - }; - err_ref.to_js(global_this) + BodyTag::Error => { + let err_js = { + // SAFETY: exclusive borrow; ends before `use_()` below. + let BodyValue::Error(err_ref) = (unsafe { &mut *body_value }) else { + unreachable!() }; - destination_blob.detach(); - // SAFETY: exclusive borrow scoped to the call; no other - // borrow of the body value is live. - let _ = unsafe { (*body_value).use_() }; - Ok(ControlFlow::Break( - JSPromise::dangerously_create_rejected_promise_value_without_notifying_vm( - global_this, err_js, - ), + err_ref.to_js(global_this) + }; + destination_blob.detach(); + // SAFETY: exclusive borrow scoped to the call; no other + // borrow of the body value is live. + let _ = unsafe { (*body_value).use_() }; + Ok(ControlFlow::Break( + JSPromise::rejected_promise(global_this, err_js).to_js(), )) - } - BodyTag::Locked => { - if destination_blob.is_s3() { - let dest_store = destination_blob - .store() - .expect("infallible: store present") - .clone(); - let s3 = dest_store.data.as_s3(); - let aws_options = s3 - .get_credentials_with_options(options.extra_options, global_this)?; - // SAFETY: exclusive borrow scoped to the call (may run JS). - let _ = unsafe { (*body_value).to_readable_stream(global_this) }?; - let readable_opt = get_stream(global_this).or_else(|| { - // SAFETY: re-borrow after `to_readable_stream`. - let BodyValue::Locked(locked) = (unsafe { &mut *body_value }) - else { - return None; - }; - locked.readable.get(global_this) - }); - if let Some(readable) = readable_opt { - if readable.is_disturbed(global_this) { - destination_blob.detach(); - return Err(global_this.throw_invalid_arguments(format_args!( - "ReadableStream has already been used" - ))); - } - let proxy_owned = http_proxy_href(global_this); - let proxy_url = proxy_owned.as_deref(); - return Ok(ControlFlow::Break(s3_client::upload_stream( - if options.extra_options.is_some() { - aws_options.credentials.dupe() - } else { - s3.get_credentials().dupe() - }, - s3.path(), - readable, - global_this, - aws_options.options, - aws_options.acl, - aws_options.storage_class, - destination_blob.content_type_or_mime_type(), - // SAFETY: `*const [u8]` borrows from sibling - // `_*_slice` fields on `aws_options`, which - // outlives this call. - aws_options.content_disposition.as_deref(), - aws_options.content_encoding.as_deref(), - proxy_url, - aws_options.request_payer, - None, - core::ptr::null_mut(), - )?)); + } + BodyTag::Locked => { + if destination_blob.is_s3() { + let dest_store = destination_blob + .store() + .expect("infallible: store present") + .clone(); + let s3 = dest_store.data.as_s3(); + let aws_options = + s3.get_credentials_with_options(options.extra_options, global_this)?; + // SAFETY: exclusive borrow scoped to the call (may run JS). + let _ = unsafe { (*body_value).to_readable_stream(global_this) }?; + let readable_opt = get_stream(global_this).or_else(|| { + // SAFETY: re-borrow after `to_readable_stream`. + let BodyValue::Locked(locked) = (unsafe { &mut *body_value }) else { + return None; + }; + locked.readable.get(global_this) + }); + if let Some(readable) = readable_opt { + if readable.is_disturbed(global_this) { + destination_blob.detach(); + return Err(global_this.throw_invalid_arguments(format_args!( + "ReadableStream has already been used" + ))); } - destination_blob.detach(); - return Err(global_this.throw_invalid_arguments(format_args!( - "ReadableStream has already been used" - ))); - } - let task = - bun_core::heap::into_raw(Box::new(WriteFileWaitFromLockedValueTask { - global_this: bun_ptr::BackRef::new(global_this), - // Move `destination_blob` by value into the task. - file_blob: core::mem::replace( - &mut destination_blob, - Blob::init_empty(global_this), - ), - promise: jsc::JSPromiseStrong::init(global_this), - mkdirp_if_not_exists: options.mkdirp_if_not_exists.unwrap_or(true), - })); - // SAFETY: re-borrow after the early-return paths. - let BodyValue::Locked(locked) = (unsafe { &mut *body_value }) else { - unreachable!() - }; - if let (Some(on_start_buffering), Some(orig_task)) = - (locked.on_start_buffering.take(), locked.task) - { - on_start_buffering(orig_task); + let proxy_owned = http_proxy_href(global_this); + let proxy_url = proxy_owned.as_deref(); + return Ok(ControlFlow::Break(s3_client::upload_stream( + if options.extra_options.is_some() { + aws_options.credentials.dupe() + } else { + s3.get_credentials().dupe() + }, + s3.path(), + readable, + global_this, + aws_options.options, + aws_options.acl, + aws_options.storage_class, + destination_blob.content_type_or_mime_type(), + // SAFETY: `*const [u8]` borrows from sibling + // `_*_slice` fields on `aws_options`, which + // outlives this call. + aws_options.content_disposition.as_deref(), + aws_options.content_encoding.as_deref(), + proxy_url, + aws_options.request_payer, + None, + core::ptr::null_mut(), + )?)); } - locked.task = Some(NonNull::new(task).unwrap().cast::()); - locked.on_receive_value = Some(WriteFileWaitFromLockedValueTask::then_wrap); - // SAFETY: `task` was just heap-allocated; consumed in `then_wrap`. - Ok(ControlFlow::Break(unsafe { (*task).promise.value() })) + destination_blob.detach(); + return Err(global_this.throw_invalid_arguments(format_args!( + "ReadableStream has already been used" + ))); } + let task = + bun_core::heap::into_raw(Box::new(WriteFileWaitFromLockedValueTask { + global_this: bun_ptr::BackRef::new(global_this), + // Move `destination_blob` by value into the task. + file_blob: core::mem::replace( + &mut destination_blob, + Blob::init_empty(global_this), + ), + promise: jsc::JSPromiseStrong::init(global_this), + mkdirp_if_not_exists: options.mkdirp_if_not_exists.unwrap_or(true), + })); + // SAFETY: re-borrow after the early-return paths. + let BodyValue::Locked(locked) = (unsafe { &mut *body_value }) else { + unreachable!() + }; + if let (Some(on_start_buffering), Some(orig_task)) = + (locked.on_start_buffering.take(), locked.task) + { + on_start_buffering(orig_task); + } + locked.task = Some(NonNull::new(task).unwrap().cast::()); + locked.on_receive_value = Some(WriteFileWaitFromLockedValueTask::then_wrap); + // SAFETY: `task` was just heap-allocated; consumed in `then_wrap`. + Ok(ControlFlow::Break(unsafe { (*task).promise.value() })) } - }; + } + }; // `as_class_ref` is the safe shared-borrow downcast (one audited unsafe // in `JSValue`); `get_body_value` / `get_body_readable_stream` both @@ -5418,10 +5385,11 @@ fn write_string_to_file_fast( *needs_async = true; return JSValue::ZERO; } - return JSPromise::dangerously_create_rejected_promise_value_without_notifying_vm( + return JSPromise::rejected_promise( global_this, err.with_path(pathlike.path().slice()).to_js(global_this), - ); + ) + .to_js(); } } }; @@ -5466,10 +5434,7 @@ fn write_string_to_file_fast( } else { err.with_path(pathlike.path().slice()).to_js(global_this) }; - return JSPromise::dangerously_create_rejected_promise_value_without_notifying_vm( - global_this, - err_js, - ); + return JSPromise::rejected_promise(global_this, err_js).to_js(); } } } @@ -5506,10 +5471,11 @@ fn write_bytes_to_file_fast( *_needs_async = true; return JSValue::ZERO; } - return JSPromise::dangerously_create_rejected_promise_value_without_notifying_vm( + return JSPromise::rejected_promise( global_this, err.with_path(pathlike.path().slice()).to_js(global_this), - ); + ) + .to_js(); } } }; @@ -5541,10 +5507,7 @@ fn write_bytes_to_file_fast( } else { err.with_path(pathlike.path().slice()).to_js(global_this) }; - return JSPromise::dangerously_create_rejected_promise_value_without_notifying_vm( - global_this, - err_js, - ); + return JSPromise::rejected_promise(global_this, err_js).to_js(); } } } diff --git a/src/runtime/webcore/ByteStream.rs b/src/runtime/webcore/ByteStream.rs index f5819771f3a8..1b29011f48c9 100644 --- a/src/runtime/webcore/ByteStream.rs +++ b/src/runtime/webcore/ByteStream.rs @@ -744,12 +744,7 @@ impl ByteStream { b.clear(); b.shrink_to_fit(); }); - return Ok( - jsc::JSPromise::dangerously_create_rejected_promise_value_without_notifying_vm( - global_this, - err_js, - ), - ); + return Ok(jsc::JSPromise::rejected_promise(global_this, err_js).to_js()); } if let Some(blob_) = self.to_any_blob() { diff --git a/test/js/bun/http/bun-serve-fetch-invalid-args.test.ts b/test/js/bun/http/bun-serve-fetch-invalid-args.test.ts index 7e7cae49ea0d..3d66d959d257 100644 --- a/test/js/bun/http/bun-serve-fetch-invalid-args.test.ts +++ b/test/js/bun/http/bun-serve-fetch-invalid-args.test.ts @@ -1,4 +1,5 @@ -import { expect, test } from "bun:test"; +import { describe, expect, test } from "bun:test"; +import { bunEnv, bunExe } from "harness"; test("server.fetch should reject invalid argument types without crashing", async () => { using server = Bun.serve({ @@ -16,3 +17,126 @@ test("server.fetch should reject invalid argument types without crashing", async // @ts-expect-error await expect(server.fetch(1)).rejects.toThrow("fetch() expects a string, but received Number"); }); + +test("server.fetch rejects with the value thrown by the fetch handler", async () => { + const error = Object.assign(new Error("handler threw"), { code: "E_HANDLER" }); + using server = Bun.serve({ + port: 0, + fetch() { + throw error; + }, + }); + await expect(server.fetch("/")).rejects.toBe(error); + + using serverThrowingString = Bun.serve({ + port: 0, + fetch() { + throw "not an error"; + }, + }); + await expect(serverThrowingString.fetch("/")).rejects.toBe("not an error"); +}); + +test("server.fetch rejects with an Error returned by the fetch handler", async () => { + const error = new RangeError("handler returned an error"); + using server = Bun.serve({ + port: 0, + fetch: (() => error) as any, + }); + await expect(server.fetch("/")).rejects.toBe(error); +}); + +test("server.fetch rejects instead of throwing when the body cannot be converted", async () => { + using server = Bun.serve({ + port: 0, + fetch() { + return new Response("Hello World!"); + }, + }); + const error = new Error("body getter threw"); + const body: unknown[] = []; + Object.defineProperty(body, 0, { + get() { + throw error; + }, + }); + await expect(server.fetch("/", { body: body as any })).rejects.toBe(error); +}); + +// server.fetch() returns an already-rejected promise for all of these. Like any +// other rejected promise, it has to be reported when nothing handles it. +describe.concurrent("server.fetch early rejections are tracked", () => { + const respond = `fetch() { return new Response("Hello World!"); }`; + + async function runChild(body: string) { + await using proc = Bun.spawn({ + cmd: [bunExe(), "-e", body], + env: bunEnv, + stdout: "pipe", + stderr: "pipe", + }); + const [stdout, stderr, exitCode] = await Promise.all([proc.stdout.text(), proc.stderr.text(), proc.exited]); + return { stdout, stderr, exitCode }; + } + + test.each([ + ["no arguments", respond, `server.fetch()`, "fetch() expects a string but received no arguments"], + ["a blank URL", respond, `server.fetch("")`, "fetch() URL must not be a blank string"], + ["a non-string argument", respond, `server.fetch(1)`, "fetch() expects a string, but received Number"], + [ + "a server without a fetch handler", + `routes: { "/": () => new Response("Hello World!") }`, + `server.fetch("/")`, + "fetch() requires the server to have a fetch handler", + ], + [ + "a fetch handler that throws", + `fetch() { throw new Error("handler threw"); }`, + `server.fetch("/")`, + "handler threw", + ], + [ + "a fetch handler that returns an Error", + `fetch() { return new RangeError("handler returned an error"); }`, + `server.fetch("/")`, + "handler returned an error", + ], + ["a fetch handler that returns undefined", `fetch() {}`, `server.fetch("/")`, "fetch() returned an empty value"], + [ + "a body that cannot be converted", + respond, + `const body = []; Object.defineProperty(body, 0, { get() { throw new Error("body getter threw"); } }); server.fetch("/", { body })`, + "body getter threw", + ], + ])("%s is reported as an unhandled rejection", async (_, serveOptions, call, expected) => { + const { stderr, exitCode } = await runChild(` + using server = Bun.serve({ port: 0, ${serveOptions} }); + ${call}; + `); + expect(stderr).toContain(expected); + expect(exitCode).toBe(1); + }); + + test("the returned promise is the one passed to 'unhandledRejection'", async () => { + const { stdout, stderr, exitCode } = await runChild(` + process.on("unhandledRejection", (reason, promise) => { + console.log(reason.message, promise === p); + }); + using server = Bun.serve({ port: 0, fetch() { throw new Error("handler threw"); } }); + const p = server.fetch("/"); + `); + expect(stdout).toBe("handler threw true\n"); + expect(stderr).toBe(""); + expect(exitCode).toBe(0); + }); + + test("a handled rejection is not reported", async () => { + const { stdout, stderr, exitCode } = await runChild(` + using server = Bun.serve({ port: 0, ${respond} }); + server.fetch().catch(e => console.log("caught:", e.message)); + `); + expect(stdout).toBe("caught: fetch() expects a string but received no arguments.\n"); + expect(stderr).toBe(""); + expect(exitCode).toBe(0); + }); +}); diff --git a/test/js/bun/io/bun-write.test.js b/test/js/bun/io/bun-write.test.js index f02bdf4f8e17..90d479f225b0 100644 --- a/test/js/bun/io/bun-write.test.js +++ b/test/js/bun/io/bun-write.test.js @@ -944,3 +944,95 @@ int posix_fadvise(int fd, off_t offset, off_t len, int advice) { expect(f.name).toBe(filePath); }); }); + +// These writes fail before any I/O is scheduled, so the write returns a promise +// that is already rejected. Those rejections must carry the error itself and be +// reported the same way as the ones produced later by the async path. +(isWindows ? describe : describe.concurrent)("Bun.write early rejections are tracked", () => { + // The endpoint is never contacted: the options are validated first. + const s3Options = { + accessKeyId: "test", + secretAccessKey: "test", + bucket: "my_bucket", + endpoint: "http://127.0.0.1:1", + }; + const invalidS3Options = { storageClass: "INVALID_VALUE" }; + const invalidS3Message = "storageClass must be one of"; + + async function runChild(body) { + using dir = tempDir("bun-write-early-reject", { "file.txt": "x" }); + const prelude = ` + const fs = require("fs"); + const dir = ${JSON.stringify(String(dir))}; + const file = ${JSON.stringify(join(String(dir), "file.txt"))}; + const s3file = new Bun.S3Client(${JSON.stringify(s3Options)}).file("key"); + const invalidS3Options = ${JSON.stringify(invalidS3Options)}; + `; + await using proc = Bun.spawn({ + cmd: [bunExe(), "-e", prelude + body], + env: bunEnv, + stdout: "pipe", + stderr: "pipe", + }); + const [stdout, stderr, exitCode] = await Promise.all([proc.stdout.text(), proc.stderr.text(), proc.exited]); + return { stdout, stderr, exitCode }; + } + + it.each([ + ["a string written to a directory", `Bun.write(dir, "x")`, "EISDIR"], + ["a TypedArray written to a directory", `Bun.write(dir, new Uint8Array(4))`, "EISDIR"], + // Truncating a directory fails with EINVAL on Windows and EISDIR elsewhere. + ["an empty Blob written to a directory", `Bun.write(dir, new Blob([]))`, isWindows ? "EINVAL" : "EISDIR"], + // Windows reports these two as "UV_EBADF" (errno -4083), which the substring check also accepts. + ["a string written to a read-only file descriptor", `Bun.write(Bun.file(fs.openSync(file, "r")), "x")`, "EBADF"], + [ + "a TypedArray written to a read-only file descriptor", + `Bun.write(Bun.file(fs.openSync(file, "r")), new Uint8Array(4))`, + "EBADF", + ], + ["BunFile.write() on a directory", `Bun.file(dir).write("x")`, "EISDIR"], + ["an S3 write with invalid options", `s3file.write("x", invalidS3Options)`, invalidS3Message], + [ + "an S3 write of an empty Blob with invalid options", + `s3file.write(new Blob([]), invalidS3Options)`, + invalidS3Message, + ], + ])("%s is reported as an unhandled rejection", async (_, expression, expected) => { + const { stderr, exitCode } = await runChild(`${expression};`); + expect(stderr).toContain(expected); + expect(exitCode).toBe(1); + }); + + it.each([ + ["a string", () => "x"], + ["an empty Blob", () => new Blob([])], + ])("an S3 write of %s with invalid options rejects with the validation error itself", async (_, data) => { + const promise = new Bun.S3Client(s3Options).file("key").write(data(), invalidS3Options); + await expect(promise).rejects.toBeInstanceOf(TypeError); + await expect(promise).rejects.toMatchObject({ + code: "ERR_INVALID_ARG_TYPE", + message: expect.stringContaining(invalidS3Message), + }); + }); + + it("the returned promise is the one passed to 'unhandledRejection'", async () => { + const { stdout, stderr, exitCode } = await runChild(` + process.on("unhandledRejection", (reason, promise) => { + console.log(reason.code, promise === p); + }); + const p = Bun.write(dir, "x"); + `); + expect(stdout).toBe("EISDIR true\n"); + expect(stderr).toBe(""); + expect(exitCode).toBe(0); + }); + + it("a handled rejection is not reported", async () => { + const { stdout, stderr, exitCode } = await runChild(` + Bun.write(dir, "x").catch(e => console.log("caught", e.code)); + `); + expect(stdout).toBe("caught EISDIR\n"); + expect(stderr).toBe(""); + expect(exitCode).toBe(0); + }); +}); diff --git a/test/js/bun/resolve/resolve-error.test.ts b/test/js/bun/resolve/resolve-error.test.ts index c608c2800a83..dd8be583fe5a 100644 --- a/test/js/bun/resolve/resolve-error.test.ts +++ b/test/js/bun/resolve/resolve-error.test.ts @@ -283,3 +283,47 @@ describe.concurrent("tsconfig paths wildcard with overlapping prefix/suffix", () await run("xy*xy", "xy"); }); }); + +// Bun.resolve() resolves synchronously and returns an already-settled promise. +// A rejected one has to be reported like any other unhandled rejection. +describe.concurrent("Bun.resolve() rejections are tracked", () => { + async function run(body: string) { + using dir = tempDir("bun-resolve-unhandled", {}); + await using proc = Bun.spawn({ + cmd: [bunExe(), "-e", `const dir = ${JSON.stringify(String(dir))};\n${body}`], + env: bunEnv, + stdout: "pipe", + stderr: "pipe", + }); + const [stdout, stderr, exitCode] = await Promise.all([proc.stdout.text(), proc.stderr.text(), proc.exited]); + return { stdout, stderr, exitCode }; + } + + it("an unhandled rejection is reported", async () => { + const { stdout, stderr, exitCode } = await run(`Bun.resolve("./does-not-exist", dir);`); + expect(stdout).toBe(""); + expect(stderr).toContain("Cannot find module './does-not-exist'"); + expect(exitCode).toBe(1); + }); + + it("the returned promise is the one passed to 'unhandledRejection'", async () => { + const { stdout, stderr, exitCode } = await run(` + process.on("unhandledRejection", (reason, promise) => { + console.log(reason.code, promise === p); + }); + const p = Bun.resolve("./does-not-exist", dir); + `); + expect(stdout).toBe("ERR_MODULE_NOT_FOUND true\n"); + expect(stderr).toBe(""); + expect(exitCode).toBe(0); + }); + + it("a handled rejection is not reported", async () => { + const { stdout, stderr, exitCode } = await run(` + Bun.resolve("./does-not-exist", dir).catch(e => console.log("caught", e.code)); + `); + expect(stdout).toBe("caught ERR_MODULE_NOT_FOUND\n"); + expect(stderr).toBe(""); + expect(exitCode).toBe(0); + }); +});