Skip to content

Commit

Permalink
refactor: FeatureChecker integration in ext/ crates (denoland#20797)
Browse files Browse the repository at this point in the history
  • Loading branch information
bartlomieju committed Oct 12, 2023
1 parent 5dd010a commit c464cd7
Show file tree
Hide file tree
Showing 20 changed files with 125 additions and 45 deletions.
12 changes: 6 additions & 6 deletions Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 1 addition & 1 deletion Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,7 @@ repository = "https://github.com/denoland/deno"
[workspace.dependencies]
deno_ast = { version = "0.29.3", features = ["transpiling"] }

deno_core = { version = "0.221.0" }
deno_core = { version = "0.222.0" }

deno_runtime = { version = "0.128.0", path = "./runtime" }
napi_sym = { version = "0.50.0", path = "./cli/napi/sym" }
Expand Down
18 changes: 18 additions & 0 deletions cli/factory.rs
Original file line number Diff line number Diff line change
Expand Up @@ -47,6 +47,7 @@ use crate::worker::CliMainWorkerOptions;

use deno_core::error::AnyError;
use deno_core::parking_lot::Mutex;
use deno_core::FeatureChecker;

use deno_graph::GraphKind;
use deno_runtime::deno_fs;
Expand Down Expand Up @@ -161,6 +162,7 @@ struct CliFactoryServices {
type_checker: Deferred<Arc<TypeChecker>>,
cjs_resolutions: Deferred<Arc<CjsResolutionStore>>,
cli_node_resolver: Deferred<Arc<CliNodeResolver>>,
feature_checker: Deferred<Arc<FeatureChecker>>,
}

pub struct CliFactory {
Expand Down Expand Up @@ -558,6 +560,21 @@ impl CliFactory {
.await
}

pub fn feature_checker(&self) -> &Arc<FeatureChecker> {
self.services.feature_checker.get_or_init(|| {
let mut checker = FeatureChecker::default();
checker.set_exit_cb(Box::new(crate::unstable_exit_cb));
// TODO(bartlomieju): enable, once we deprecate `--unstable` in favor
// of granular --unstable-* flags.
// feature_checker.set_warn_cb(Box::new(crate::unstable_warn_cb));
if self.options.unstable() {
checker.enable_legacy_unstable();
}

Arc::new(checker)
})
}

pub async fn create_compile_binary_writer(
&self,
) -> Result<DenoCompileBinaryWriter, AnyError> {
Expand Down Expand Up @@ -602,6 +619,7 @@ impl CliFactory {
self.fs().clone(),
self.maybe_inspector_server().clone(),
self.maybe_lockfile().clone(),
self.feature_checker().clone(),
self.create_cli_main_worker_options()?,
))
}
Expand Down
13 changes: 13 additions & 0 deletions cli/main.rs
Original file line number Diff line number Diff line change
Expand Up @@ -256,6 +256,19 @@ fn unwrap_or_exit<T>(result: Result<T, AnyError>) -> T {
}
}

pub(crate) fn unstable_exit_cb(_feature: &str, api_name: &str) {
// TODO(bartlomieju): change to "The `--unstable-{feature}` flag must be provided.".
eprintln!("Unstable API '{api_name}'. The --unstable flag must be provided.");
std::process::exit(70);
}

#[allow(dead_code)]
pub(crate) fn unstable_warn_cb(feature: &str) {
eprintln!(
"The `--unstable` flag is deprecated, use --unstable-{feature} instead."
);
}

pub fn main() {
setup_panic_hook();

Expand Down
13 changes: 13 additions & 0 deletions cli/standalone/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,7 @@ use deno_core::error::type_error;
use deno_core::error::AnyError;
use deno_core::futures::FutureExt;
use deno_core::v8_set_flags;
use deno_core::FeatureChecker;
use deno_core::ModuleLoader;
use deno_core::ModuleSpecifier;
use deno_core::ModuleType;
Expand Down Expand Up @@ -424,6 +425,17 @@ pub async fn run(

PermissionsContainer::new(Permissions::from_options(&permissions)?)
};
let feature_checker = Arc::new({
let mut checker = FeatureChecker::default();
checker.set_exit_cb(Box::new(crate::unstable_exit_cb));
// TODO(bartlomieju): enable, once we deprecate `--unstable` in favor
// of granular --unstable-* flags.
// feature_checker.set_warn_cb(Box::new(crate::unstable_warn_cb));
if metadata.unstable {
checker.enable_legacy_unstable();
}
checker
});
let worker_factory = CliMainWorkerFactory::new(
StorageKeyResolver::empty(),
npm_resolver,
Expand All @@ -434,6 +446,7 @@ pub async fn run(
fs,
None,
None,
feature_checker,
CliMainWorkerOptions {
argv: metadata.argv,
log_level: WorkerLogLevel::Info,
Expand Down
2 changes: 1 addition & 1 deletion cli/tsc/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -948,7 +948,7 @@ mod tests {
.context("Unable to get CWD")
.unwrap(),
);
let mut op_state = OpState::new(1);
let mut op_state = OpState::new(1, None);
op_state.put(state);
op_state
}
Expand Down
6 changes: 6 additions & 0 deletions cli/worker.rs
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@ use deno_core::url::Url;
use deno_core::v8;
use deno_core::CompiledWasmModuleStore;
use deno_core::Extension;
use deno_core::FeatureChecker;
use deno_core::ModuleId;
use deno_core::ModuleLoader;
use deno_core::SharedArrayBufferStore;
Expand Down Expand Up @@ -109,6 +110,7 @@ struct SharedWorkerState {
fs: Arc<dyn deno_fs::FileSystem>,
maybe_inspector_server: Option<Arc<InspectorServer>>,
maybe_lockfile: Option<Arc<Mutex<Lockfile>>>,
feature_checker: Arc<FeatureChecker>,
}

impl SharedWorkerState {
Expand Down Expand Up @@ -313,6 +315,7 @@ impl CliMainWorkerFactory {
fs: Arc<dyn deno_fs::FileSystem>,
maybe_inspector_server: Option<Arc<InspectorServer>>,
maybe_lockfile: Option<Arc<Mutex<Lockfile>>>,
feature_checker: Arc<FeatureChecker>,
options: CliMainWorkerOptions,
) -> Self {
Self {
Expand All @@ -330,6 +333,7 @@ impl CliMainWorkerFactory {
fs,
maybe_inspector_server,
maybe_lockfile,
feature_checker,
}),
}
}
Expand Down Expand Up @@ -510,6 +514,7 @@ impl CliMainWorkerFactory {
shared.compiled_wasm_module_store.clone(),
),
stdio,
feature_checker: shared.feature_checker.clone(),
};

let worker = MainWorker::bootstrap_from_options(
Expand Down Expand Up @@ -681,6 +686,7 @@ fn create_web_worker_callback(
),
stdio: stdio.clone(),
cache_storage_dir,
feature_checker: shared.feature_checker.clone(),
};

WebWorker::bootstrap_from_options(
Expand Down
11 changes: 8 additions & 3 deletions ext/broadcast_channel/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,8 @@ use deno_core::OpState;
use deno_core::Resource;
use deno_core::ResourceId;

pub const UNSTABLE_FEATURE_NAME: &str = "broadcast-channel";

#[async_trait]
pub trait BroadcastChannel: Clone {
type Resource: Resource;
Expand Down Expand Up @@ -48,9 +50,12 @@ pub fn op_broadcast_subscribe<BC>(
where
BC: BroadcastChannel + 'static,
{
state
.feature_checker
.check_legacy_unstable_or_exit("BroadcastChannel");
// TODO(bartlomieju): replace with `state.feature_checker.check_or_exit`
// once we phase out `check_or_exit_with_legacy_fallback`
state.feature_checker.check_or_exit_with_legacy_fallback(
UNSTABLE_FEATURE_NAME,
"BroadcastChannel",
);
let bc = state.borrow::<BC>();
let resource = bc.subscribe()?;
Ok(state.resource_table.add(resource))
Expand Down
6 changes: 5 additions & 1 deletion ext/ffi/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -45,10 +45,14 @@ const _: () = {
pub(crate) const MAX_SAFE_INTEGER: isize = 9007199254740991;
pub(crate) const MIN_SAFE_INTEGER: isize = -9007199254740991;

pub const UNSTABLE_FEATURE_NAME: &str = "ffi";

fn check_unstable(state: &OpState, api_name: &str) {
// TODO(bartlomieju): replace with `state.feature_checker.check_or_exit`
// once we phase out `check_or_exit_with_legacy_fallback`
state
.feature_checker
.check_legacy_unstable_or_exit(api_name);
.check_or_exit_with_legacy_fallback(UNSTABLE_FEATURE_NAME, api_name)
}

pub trait FfiPermissions {
Expand Down
6 changes: 5 additions & 1 deletion ext/fs/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -64,11 +64,15 @@ pub trait FsPermissions {
}
}

pub const UNSTABLE_FEATURE_NAME: &str = "fs";

/// Helper for checking unstable features. Used for sync ops.
fn check_unstable(state: &OpState, api_name: &str) {
// TODO(bartlomieju): replace with `state.feature_checker.check_or_exit`
// once we phase out `check_or_exit_with_legacy_fallback`
state
.feature_checker
.check_legacy_unstable_or_exit(api_name);
.check_or_exit_with_legacy_fallback(UNSTABLE_FEATURE_NAME, api_name);
}

deno_core::extension!(deno_fs,
Expand Down
10 changes: 9 additions & 1 deletion ext/http/http_next.rs
Original file line number Diff line number Diff line change
Expand Up @@ -83,6 +83,8 @@ static USE_WRITEV: Lazy<bool> = Lazy::new(|| {
false
});

pub const UNSTABLE_FEATURE_NAME: &str = "http";

/// All HTTP/2 connections start with this byte string.
///
/// In HTTP/2, each endpoint is required to send a connection preface as a final confirmation
Expand Down Expand Up @@ -1105,10 +1107,16 @@ pub async fn op_http_close(
.take::<HttpJoinHandle>(rid)?;

if graceful {
// TODO(bartlomieju): replace with `state.feature_checker.check_or_exit`
// once we phase out `check_or_exit_with_legacy_fallback`
state
.borrow()
.feature_checker
.check_legacy_unstable_or_exit("Deno.Server.shutdown");
.check_or_exit_with_legacy_fallback(
UNSTABLE_FEATURE_NAME,
"Deno.Server.shutdown",
);

// In a graceful shutdown, we close the listener and allow all the remaining connections to drain
join_handle.listen_cancel_handle().cancel();
} else {
Expand Down
6 changes: 5 additions & 1 deletion ext/kv/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,8 @@ use serde::Serialize;

pub use crate::interface::*;

pub const UNSTABLE_FEATURE_NAME: &str = "kv";

const MAX_WRITE_KEY_SIZE_BYTES: usize = 2048;
// range selectors can contain 0x00 or 0xff suffixes
const MAX_READ_KEY_SIZE_BYTES: usize = MAX_WRITE_KEY_SIZE_BYTES + 1;
Expand Down Expand Up @@ -89,9 +91,11 @@ where
{
let handler = {
let state = state.borrow();
// TODO(bartlomieju): replace with `state.feature_checker.check_or_exit`
// once we phase out `check_or_exit_with_legacy_fallback`
state
.feature_checker
.check_legacy_unstable_or_exit("Deno.openKv");
.check_or_exit_with_legacy_fallback(UNSTABLE_FEATURE_NAME, "Deno.openKv");
state.borrow::<Rc<DBH>>().clone()
};
let db = handler.open(state.clone(), path).await?;
Expand Down
6 changes: 5 additions & 1 deletion ext/net/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,8 @@ use std::path::Path;
use std::path::PathBuf;
use std::sync::Arc;

pub const UNSTABLE_FEATURE_NAME: &str = "net";

pub trait NetPermissions {
fn check_net<T: AsRef<str>>(
&mut self,
Expand All @@ -29,9 +31,11 @@ pub trait NetPermissions {

/// Helper for checking unstable features. Used for sync ops.
fn check_unstable(state: &OpState, api_name: &str) {
// TODO(bartlomieju): replace with `state.feature_checker.check_or_exit`
// once we phase out `check_or_exit_with_legacy_fallback`
state
.feature_checker
.check_legacy_unstable_or_exit(api_name);
.check_or_exit_with_legacy_fallback(UNSTABLE_FEATURE_NAME, api_name);
}

pub fn get_declaration() -> PathBuf {
Expand Down
8 changes: 4 additions & 4 deletions ext/net/ops.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1041,16 +1041,16 @@ mod tests {
}
);

let mut feature_checker = deno_core::FeatureChecker::default();
feature_checker.enable_legacy_unstable();

let mut runtime = JsRuntime::new(RuntimeOptions {
extensions: vec![test_ext::init_ops()],
feature_checker: Some(Arc::new(feature_checker)),
..Default::default()
});

let conn_state = runtime.op_state();
conn_state
.borrow_mut()
.feature_checker
.enable_legacy_unstable();

let server_addr: Vec<&str> = clone_addr.split(':').collect();
let ip_addr = IpAddr {
Expand Down
4 changes: 3 additions & 1 deletion runtime/ops/http.rs
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,8 @@ use deno_net::io::UnixStreamResource;
#[cfg(unix)]
use tokio::net::UnixStream;

pub const UNSTABLE_FEATURE_NAME: &str = "http";

deno_core::extension!(
deno_http_runtime,
ops = [op_http_start, op_http_upgrade],
Expand Down Expand Up @@ -73,7 +75,7 @@ fn op_http_start(
.resource_table
.take::<deno_net::io::UnixStreamResource>(tcp_stream_rid)
{
super::check_unstable(state, "Deno.serveHttp");
super::check_unstable(state, UNSTABLE_FEATURE_NAME, "Deno.serveHttp");

// This UNIX socket might be used somewhere else. If it's the case, we cannot proceed with the
// process of starting a HTTP server on top of this UNIX socket, so we just return a bad
Expand Down
6 changes: 4 additions & 2 deletions runtime/ops/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -15,10 +15,12 @@ pub mod worker_host;
use deno_core::OpState;

/// Helper for checking unstable features. Used for sync ops.
pub fn check_unstable(state: &OpState, api_name: &str) {
pub fn check_unstable(state: &OpState, feature: &str, api_name: &str) {
// TODO(bartlomieju): replace with `state.feature_checker.check_or_exit`
// once we phase out `check_or_exit_with_legacy_fallback`
state
.feature_checker
.check_legacy_unstable_or_exit(api_name);
.check_or_exit_with_legacy_fallback(feature, api_name);
}

pub struct TestingFeaturesEnabled(pub bool);
Loading

0 comments on commit c464cd7

Please sign in to comment.