Skip to content

fix(turbopack): Allow google font fetch errors to propagate when in production #79999

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
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
2 changes: 1 addition & 1 deletion crates/next-core/src/next_client/context.rs
Original file line number Diff line number Diff line change
Expand Up @@ -161,7 +161,7 @@ pub async fn get_client_resolve_options_context(
execution_context: Vc<ExecutionContext>,
) -> Result<Vc<ResolveOptionsContext>> {
let next_client_import_map =
get_next_client_import_map(*project_path, ty, next_config, execution_context)
get_next_client_import_map(*project_path, ty, next_config, mode, execution_context)
.to_resolved()
.await?;
let next_client_fallback_import_map = get_next_client_fallback_import_map(ty)
Expand Down
2 changes: 1 addition & 1 deletion crates/next-core/src/next_edge/context.rs
Original file line number Diff line number Diff line change
Expand Up @@ -110,7 +110,7 @@ pub async fn get_edge_resolve_options_context(
execution_context: Vc<ExecutionContext>,
) -> Result<Vc<ResolveOptionsContext>> {
let next_edge_import_map =
get_next_edge_import_map(*project_path, ty, next_config, execution_context)
get_next_edge_import_map(*project_path, ty, next_config, mode, execution_context)
.to_resolved()
.await?;

Expand Down
74 changes: 59 additions & 15 deletions crates/next-core/src/next_font/google/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@ use turbopack_core::{
asset::AssetContent,
context::AssetContext,
ident::AssetIdent,
issue::{IssueExt, IssueSeverity},
issue::{IssueExt, IssueSeverity, StyledString},
reference_type::{InnerAssets, ReferenceType},
resolve::{
ResolveResult,
Expand Down Expand Up @@ -48,7 +48,8 @@ use super::{
},
};
use crate::{
embed_js::next_js_file_path, next_app::metadata::split_extension, util::load_next_js_templateon,
embed_js::next_js_file_path, mode::NextMode, next_app::metadata::split_extension,
next_font::issue::NextFontIssue, util::load_next_js_templateon,
};

pub mod font_fallback;
Expand Down Expand Up @@ -179,6 +180,7 @@ impl ImportMappingReplacement for NextFontGoogleReplacer {
pub struct NextFontGoogleCssModuleReplacer {
project_path: ResolvedVc<FileSystemPath>,
execution_context: ResolvedVc<ExecutionContext>,
next_mode: ResolvedVc<NextMode>,
}

#[turbo_tasks::value_impl]
Expand All @@ -187,10 +189,12 @@ impl NextFontGoogleCssModuleReplacer {
pub fn new(
project_path: ResolvedVc<FileSystemPath>,
execution_context: ResolvedVc<ExecutionContext>,
next_mode: ResolvedVc<NextMode>,
) -> Vc<Self> {
Self::cell(NextFontGoogleCssModuleReplacer {
project_path,
execution_context,
next_mode,
})
}

Expand All @@ -215,6 +219,7 @@ impl NextFontGoogleCssModuleReplacer {
// requests to Google Fonts.
let env = Vc::upcast::<Box<dyn ProcessEnv>>(CommandLineProcessEnv::new());
let mocked_responses_path = &*env.read("NEXT_FONT_GOOGLE_MOCKED_RESPONSES".into()).await?;

let stylesheet_str = mocked_responses_path
.as_ref()
.map_or_else(
Expand All @@ -224,7 +229,6 @@ impl NextFontGoogleCssModuleReplacer {
.await?;

let font_fallback = get_font_fallback(*self.project_path, options);

let stylesheet = match stylesheet_str {
Some(s) => Some(
update_google_stylesheet(
Expand All @@ -237,10 +241,57 @@ impl NextFontGoogleCssModuleReplacer {
.await?,
),
None => {
println!(
"Failed to download `{}` from Google Fonts. Using fallback font instead.",
options.await?.font_family
);
match *self.next_mode.await? {
// If we're in production mode, we want to fail the build to ensure proper font
// rendering.
NextMode::Build => {
NextFontIssue {
path: css_virtual_path.to_resolved().await?,
title: StyledString::Line(vec![
StyledString::Code("next/font:".into()),
StyledString::Text(" error:".into()),
])
.resolved_cell(),
description: StyledString::Text(
format!(
"Failed to fetch `{}` from Google Fonts.",
options.await?.font_family
)
.into(),
)
.resolved_cell(),
severity: IssueSeverity::Error.resolved_cell(),
}
.resolved_cell()
.emit();
}
// Inform the user of the failure to retreive the stylesheet / font, but don't
// propagate this error. We don't want e.g. offline connections to prevent page
// renders during development.
NextMode::Development => {
NextFontIssue {
path: css_virtual_path.to_resolved().await?,
title: StyledString::Line(vec![
StyledString::Code("next/font:".into()),
StyledString::Text(" warning:".into()),
])
.resolved_cell(),
description: StyledString::Text(
format!(
"Failed to download `{}` from Google Fonts. Using fallback \
font instead.",
options.await?.font_family
)
.into(),
)
.resolved_cell(),
severity: IssueSeverity::Warning.resolved_cell(),
}
.resolved_cell()
.emit();
}
}

None
}
};
Expand Down Expand Up @@ -610,16 +661,9 @@ async fn fetch_from_google_fonts(
)
.await?;

Ok(match &*result {
Ok(match *result {
Ok(r) => Some(*r.await?.body),
Err(err) => {
// Inform the user of the failure to retreive the stylesheet / font, but don't
// propagate this error. We don't want e.g. offline connections to prevent page
// renders during development. During production builds, however, this error
// should propagate.
//
// TODO(WEB-283): Use fallback in dev in this case
// TODO(WEB-293): Fail production builds (not dev) in this case
err.to_issue(IssueSeverity::Warning.into(), virtual_path)
.to_resolved()
.await?
Expand Down
9 changes: 8 additions & 1 deletion crates/next-core/src/next_import_map.rs
Original file line number Diff line number Diff line change
Expand Up @@ -93,6 +93,7 @@ pub async fn get_next_client_import_map(
project_path: ResolvedVc<FileSystemPath>,
ty: Value<ClientContextType>,
next_config: Vc<NextConfig>,
next_mode: Vc<NextMode>,
execution_context: Vc<ExecutionContext>,
) -> Result<Vc<ImportMap>> {
let mut import_map = ImportMap::empty();
Expand All @@ -102,6 +103,7 @@ pub async fn get_next_client_import_map(
project_path,
execution_context,
next_config,
next_mode,
false,
)
.await?;
Expand Down Expand Up @@ -289,6 +291,7 @@ pub async fn get_next_server_import_map(
project_path: ResolvedVc<FileSystemPath>,
ty: Value<ServerContextType>,
next_config: Vc<NextConfig>,
next_mode: Vc<NextMode>,
execution_context: Vc<ExecutionContext>,
) -> Result<Vc<ImportMap>> {
let mut import_map = ImportMap::empty();
Expand All @@ -298,6 +301,7 @@ pub async fn get_next_server_import_map(
project_path,
execution_context,
next_config,
next_mode,
false,
)
.await?;
Expand Down Expand Up @@ -384,6 +388,7 @@ pub async fn get_next_edge_import_map(
project_path: ResolvedVc<FileSystemPath>,
ty: Value<ServerContextType>,
next_config: Vc<NextConfig>,
next_mode: Vc<NextMode>,
execution_context: Vc<ExecutionContext>,
) -> Result<Vc<ImportMap>> {
let mut import_map = ImportMap::empty();
Expand Down Expand Up @@ -435,6 +440,7 @@ pub async fn get_next_edge_import_map(
project_path,
execution_context,
next_config,
next_mode,
true,
)
.await?;
Expand Down Expand Up @@ -845,6 +851,7 @@ async fn insert_next_shared_aliases(
project_path: ResolvedVc<FileSystemPath>,
execution_context: Vc<ExecutionContext>,
next_config: Vc<NextConfig>,
next_mode: Vc<NextMode>,
is_runtime_edge: bool,
) -> Result<()> {
let package_root = next_js_fs().root().to_resolved().await?;
Expand Down Expand Up @@ -892,7 +899,7 @@ async fn insert_next_shared_aliases(
import_map.insert_alias(
AliasPattern::exact("@vercel/turbopack-next/internal/font/google/cssmodule.module.css"),
ImportMapping::Dynamic(ResolvedVc::upcast(
NextFontGoogleCssModuleReplacer::new(*project_path, execution_context)
NextFontGoogleCssModuleReplacer::new(*project_path, execution_context, next_mode)
.to_resolved()
.await?,
))
Expand Down
2 changes: 1 addition & 1 deletion crates/next-core/src/next_server/context.rs
Original file line number Diff line number Diff line change
Expand Up @@ -134,7 +134,7 @@ pub async fn get_server_resolve_options_context(
execution_context: Vc<ExecutionContext>,
) -> Result<Vc<ResolveOptionsContext>> {
let next_server_import_map =
get_next_server_import_map(*project_path, ty, next_config, execution_context)
get_next_server_import_map(*project_path, ty, next_config, mode, execution_context)
.to_resolved()
.await?;
let foreign_code_context_condition =
Expand Down
4 changes: 2 additions & 2 deletions test/turbopack-build-tests-manifest.json
Original file line number Diff line number Diff line change
Expand Up @@ -7745,8 +7745,8 @@
"runtimeError": false
},
"test/e2e/next-font/google-fetch-error.test.ts": {
"passed": [],
"failed": ["next/font/google fetch error should error when not in dev"],
"passed": ["next/font/google fetch error should error when not in dev"],
"failed": [],
"pending": [],
"flakey": [],
"runtimeError": false
Expand Down
Loading