Skip to content

Commit

Permalink
fix(microfrontends): fix handling of unsupported version (#9883)
Browse files Browse the repository at this point in the history
### Description

Make it so any invalid MFE configs no longer fail the process. 

We had this behavior previously for unsupported versions, but it broke
somewhere along the lines where we were throwing `InvalidVersions`
instead.

### Testing Instructions

Added unit test. Quick manual verification:
```
[0 olszewski@chriss-mbp] /Users/olszewski/code/vapi $ turbo_dev --skip-infer build --dry > /dev/null
turbo 2.4.0

 WARNING  Ignoring control-plane-default: Unsupported microfrontends configuration version: 2. Supported versions: ["1"]
```
  • Loading branch information
chris-olszewski authored Feb 3, 2025
1 parent 0dc72f6 commit 478b66d
Show file tree
Hide file tree
Showing 4 changed files with 46 additions and 33 deletions.
9 changes: 7 additions & 2 deletions crates/turborepo-lib/src/run/builder.rs
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ use std::{
};

use chrono::Local;
use tracing::debug;
use tracing::{debug, warn};
use turbopath::{AbsoluteSystemPath, AbsoluteSystemPathBuf};
use turborepo_analytics::{start_analytics, AnalyticsHandle, AnalyticsSender};
use turborepo_api_client::{APIAuth, APIClient};
Expand Down Expand Up @@ -363,7 +363,12 @@ impl RunBuilder {
repo_telemetry.track_size(pkg_dep_graph.len());
run_telemetry.track_run_type(self.opts.run_opts.dry_run.is_some());
let micro_frontend_configs =
MicrofrontendsConfigs::from_disk(&self.repo_root, &pkg_dep_graph)?;
MicrofrontendsConfigs::from_disk(&self.repo_root, &pkg_dep_graph)
.inspect_err(|err| {
warn!("Ignoring invalid microfrontends configuration: {err}");
})
.ok()
.flatten();

let scm = scm.await.expect("detecting scm panicked");
let async_cache = AsyncCache::new(
Expand Down
1 change: 1 addition & 0 deletions crates/turborepo-microfrontends/src/configv1.rs
Original file line number Diff line number Diff line change
Expand Up @@ -67,6 +67,7 @@ impl ConfigV1 {
Err(Error::InvalidVersion {
expected: "1",
actual: version,
path: source.to_string(),
})
}
} else {
Expand Down
3 changes: 2 additions & 1 deletion crates/turborepo-microfrontends/src/error.rs
Original file line number Diff line number Diff line change
Expand Up @@ -15,10 +15,11 @@ pub enum Error {
UnsupportedVersion(String),
#[error("Configuration references config located in package {reference}.")]
ChildConfig { reference: String },
#[error("Cannot parse config with version '{actual}' as version '{expected}'.")]
#[error("`{path}`: Cannot parse config with version '{actual}' as version '{expected}'.")]
InvalidVersion {
expected: &'static str,
actual: String,
path: String,
},
}

Expand Down
66 changes: 36 additions & 30 deletions crates/turborepo-microfrontends/src/lib.rs
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
#![feature(assert_matches)]
#![deny(clippy::all)]
mod configv1;
mod error;
Expand Down Expand Up @@ -52,15 +53,18 @@ impl Config {
package_dir: &AnchoredSystemPath,
) -> Result<Option<Self>, Error> {
let absolute_dir = repo_root.resolve(package_dir);
let mut config = if let Some(config) = Self::load_v1_dir(&absolute_dir)? {
Ok(Some(config))
} else {
Self::load_v1_dir(&absolute_dir)
// we want to try different paths and then do `from_str`
let Some((contents, path)) = Self::load_v1_dir(&absolute_dir) else {
return Ok(None);
};
if let Ok(Some(config)) = &mut config {
config.set_path(package_dir);
}
config
let contents = contents?;
let mut config = Config::from_str(&contents, path.as_str())?;
config.filename = path
.file_name()
.expect("microfrontends config should not be root")
.to_owned();
config.set_path(package_dir);
Ok(Some(config))
}

pub fn from_str(input: &str, source: &str) -> Result<Self, Error> {
Expand Down Expand Up @@ -127,35 +131,17 @@ impl Config {
}
}

fn load_v1_dir(dir: &AbsoluteSystemPath) -> Result<Option<Self>, Error> {
fn load_v1_dir(
dir: &AbsoluteSystemPath,
) -> Option<(Result<String, io::Error>, AbsoluteSystemPathBuf)> {
let load_config =
|filename: &str| -> Option<(Result<String, io::Error>, AbsoluteSystemPathBuf)> {
let path = dir.join_component(filename);
let contents = path.read_existing_to_string().transpose()?;
Some((contents, path))
};
let Some((contents, path)) = load_config(DEFAULT_MICROFRONTENDS_CONFIG_V1)
load_config(DEFAULT_MICROFRONTENDS_CONFIG_V1)
.or_else(|| load_config(DEFAULT_MICROFRONTENDS_CONFIG_V1_ALT))
else {
return Ok(None);
};
let contents = contents?;

ConfigV1::from_str(&contents, path.as_str())
.and_then(|result| match result {
configv1::ParseResult::Actual(config_v1) => Ok(Config {
inner: ConfigInner::V1(config_v1),
filename: path
.file_name()
.expect("microfrontends config should not be root")
.to_owned(),
path: None,
}),
configv1::ParseResult::Reference(default_app) => Err(Error::ChildConfig {
reference: default_app,
}),
})
.map(Some)
}

/// Sets the path the configuration was loaded from
Expand All @@ -166,6 +152,8 @@ impl Config {

#[cfg(test)]
mod test {
use std::assert_matches::assert_matches;

use insta::assert_snapshot;
use tempfile::TempDir;
use test_case::test_case;
Expand All @@ -192,6 +180,12 @@ mod test {
path.create_with_contents(r#"{"version": "1", "applications": {"web": {"development": {"task": "serve"}}, "docs": {}}}"#)
}

fn add_v2_config(dir: &AbsoluteSystemPath) -> Result<(), std::io::Error> {
let path = dir.join_component(DEFAULT_MICROFRONTENDS_CONFIG_V1);
path.ensure_dir()?;
path.create_with_contents(r#"{"version": "2", "applications": {"web": {"development": {"task": "serve"}}, "docs": {}}}"#)
}

fn add_v1_alt_config(dir: &AbsoluteSystemPath) -> Result<(), std::io::Error> {
let path = dir.join_component(DEFAULT_MICROFRONTENDS_CONFIG_V1_ALT);
path.ensure_dir()?;
Expand Down Expand Up @@ -286,4 +280,16 @@ mod test {
assert_eq!(actual_version, case.expected_version);
assert_eq!(actual_path, case.expected_path().as_deref());
}

#[test]
fn test_unsupported_version_from_dir() {
let dir = TempDir::new().unwrap();
let repo_root = AbsoluteSystemPath::new(dir.path().to_str().unwrap()).unwrap();
let pkg_dir = AnchoredSystemPath::new("web").unwrap();
let pkg_path = repo_root.resolve(pkg_dir);
add_v2_config(&pkg_path).unwrap();
let config = Config::load_from_dir(repo_root, pkg_dir);

assert_matches!(config, Err(Error::UnsupportedVersion(..)));
}
}

0 comments on commit 478b66d

Please sign in to comment.