Skip to content

Commit e42a594

Browse files
aaronvgaaronvg
authored
only print out vertex auth errors if it failed to auth completely (#1843)
<!-- ELLIPSIS_HIDDEN --> > [!IMPORTANT] > Improve error logging in `VertexAuth` and rename a TypeScript import function for clarity. > > - **Behavior**: > - Modify `VertexAuth` in `std_auth.rs` to log errors only if all authentication strategies fail. > - Collects errors in a vector and logs them collectively if no strategy succeeds. > - **Functions**: > - Rename `replace_ts_imports_with_js` to `add_js_suffix_to_imports` in `mod.rs` and test files. > - **Tests**: > - Update `vertex.test.ts` to test `TestVertex` with a specific input requiring the word "donkey". > > <sup>This description was created by </sup>[<img alt="Ellipsis" src="https://img.shields.io/badge/Ellipsis-blue?color=175173">](https://www.ellipsis.dev?ref=BoundaryML%2Fbaml&utm_source=github&utm_medium=referral)<sup> for c5bec3a. You can [customize](https://app.ellipsis.dev/BoundaryML/settings/summaries) this summary. It will automatically update as commits are pushed.</sup> <!-- ELLIPSIS_HIDDEN -->
1 parent 1657e35 commit e42a594

11 files changed

Lines changed: 51 additions & 60 deletions

File tree

‎engine/baml-runtime/src/internal/llm_client/primitive/vertex/std_auth.rs‎

Lines changed: 16 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -35,6 +35,8 @@ impl VertexAuth {
3535
}
3636
ResolvedGcpAuthStrategy::SystemDefault => {
3737
log::debug!("Attempting to auth using SystemDefault strategy (local mods)");
38+
let mut errors = Vec::new();
39+
3840
match gcp_auth::ConfigDefaultCredentials::new().await {
3941
Ok(authz_user) => {
4042
log::debug!(
@@ -43,12 +45,9 @@ impl VertexAuth {
4345
return Ok(VertexAuth::ConfigDefaultCredentials(authz_user));
4446
}
4547
Err(e) => {
46-
log::error!(
47-
"{:?}",
48-
anyhow::Error::from(e).context(
49-
"Failed to auth using GcloudApplicationDefaultCredentials strategy"
50-
)
51-
);
48+
errors.push(anyhow::Error::from(e).context(
49+
"Failed to auth using GcloudApplicationDefaultCredentials strategy",
50+
));
5251
}
5352
}
5453
match gcp_auth::MetadataServiceAccount::new().await {
@@ -57,10 +56,9 @@ impl VertexAuth {
5756
return Ok(VertexAuth::MetadataServiceAccount(authz_user));
5857
}
5958
Err(e) => {
60-
log::error!(
61-
"{:?}",
59+
errors.push(
6260
anyhow::Error::from(e)
63-
.context("Failed to auth using MetadataServiceAccount strategy")
61+
.context("Failed to auth using MetadataServiceAccount strategy"),
6462
);
6563
}
6664
}
@@ -70,15 +68,20 @@ impl VertexAuth {
7068
return Ok(VertexAuth::GCloudAuthorizedUser(authz_user));
7169
}
7270
Err(e) => {
73-
log::error!(
74-
"{:?}",
71+
errors.push(
7572
anyhow::Error::from(e)
76-
.context("Failed to auth using GCloudAuthorizedUser strategy")
73+
.context("Failed to auth using GCloudAuthorizedUser strategy"),
7774
);
7875
}
7976
}
77+
78+
// Log all collected errors if no strategy succeeded
79+
for err in &errors {
80+
log::error!("{:?}", err);
81+
}
8082
anyhow::bail!(
81-
"Failed to auth - system_default strategy did not resolve successfully"
83+
"Failed to auth - system_default strategy did not resolve successfully. Errors encountered: {:?}",
84+
errors
8285
)
8386
}
8487
}

‎engine/language_client_codegen/src/typescript/mod.rs‎

Lines changed: 21 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -293,16 +293,15 @@ pub(crate) fn generate(
293293

294294
// if it's typescriopt and generator.esm is enabled, we need to change the imports in each file to use the .js extension
295295
if generator.module_format == Some(ModuleFormat::Esm) {
296-
baml_log::info!("Changing imports to .js for ESM");
297296
collector.modify_files(|content: &mut String| {
298-
*content = replace_ts_imports_with_js(content);
297+
*content = add_js_suffix_to_imports(content);
299298
});
300299
}
301300

302301
collector.commit(&generator.output_dir())
303302
}
304303

305-
fn replace_ts_imports_with_js(content: &str) -> String {
304+
fn add_js_suffix_to_imports(content: &str) -> String {
306305
// Regex to find import/export statements with module specifiers.
307306
// It captures the import/export part, quotes, and the path itself.
308307
// Escaped curly braces in the character set just in case.
@@ -752,87 +751,87 @@ mod tests {
752751
fn test_replace_ts_imports_with_js() {
753752
// Add .js to relative paths without extension
754753
assert_eq!(
755-
replace_ts_imports_with_js("import { Foo } from './bar';"),
754+
add_js_suffix_to_imports("import { Foo } from './bar';"),
756755
"import { Foo } from './bar.js';"
757756
);
758757
assert_eq!(
759-
replace_ts_imports_with_js("export * from \"../baz/qux\";"),
758+
add_js_suffix_to_imports("export * from \"../baz/qux\";"),
760759
"export * from \"../baz/qux.js\";"
761760
);
762761
assert_eq!(
763-
replace_ts_imports_with_js("import type { Bar } from './bar'"),
762+
add_js_suffix_to_imports("import type { Bar } from './bar'"),
764763
"import type { Bar } from './bar.js'"
765764
);
766765
assert_eq!(
767-
replace_ts_imports_with_js("import {\n Thing1,\n Thing2\n} from \"./things\";"),
766+
add_js_suffix_to_imports("import {\n Thing1,\n Thing2\n} from \"./things\";"),
768767
"import {\n Thing1,\n Thing2\n} from \"./things.js\";"
769768
);
770769

771770
// Replace .ts with .js in relative paths
772771
assert_eq!(
773-
replace_ts_imports_with_js("import { Foo } from './bar.ts';"),
772+
add_js_suffix_to_imports("import { Foo } from './bar.ts';"),
774773
"import { Foo } from './bar.js';"
775774
);
776775
assert_eq!(
777-
replace_ts_imports_with_js("export * from \"../baz/qux.ts\";"),
776+
add_js_suffix_to_imports("export * from \"../baz/qux.ts\";"),
778777
"export * from \"../baz/qux.js\";"
779778
);
780779

781780
// Should ignore already correct .js paths
782781
assert_eq!(
783-
replace_ts_imports_with_js("import { Foo } from './bar.js';"),
782+
add_js_suffix_to_imports("import { Foo } from './bar.js';"),
784783
"import { Foo } from './bar.js';"
785784
);
786785
// Should ignore other extensions like .css, .mjs, .cjs
787786
assert_eq!(
788-
replace_ts_imports_with_js("import styles from './styles.css';"),
787+
add_js_suffix_to_imports("import styles from './styles.css';"),
789788
"import styles from './styles.css';"
790789
);
791790
assert_eq!(
792-
replace_ts_imports_with_js("import config from './config.json';"),
791+
add_js_suffix_to_imports("import config from './config.json';"),
793792
"import config from './config.json';"
794793
);
795794
assert_eq!(
796-
replace_ts_imports_with_js("import { util } from './util.mjs';"),
795+
add_js_suffix_to_imports("import { util } from './util.mjs';"),
797796
"import { util } from './util.mjs';"
798797
);
799798
assert_eq!(
800-
replace_ts_imports_with_js("import { main } from '../main.cjs';"),
799+
add_js_suffix_to_imports("import { main } from '../main.cjs';"),
801800
"import { main } from '../main.cjs';"
802801
);
803802
assert_eq!(
804-
replace_ts_imports_with_js("import { Comp } from './Comp.tsx';"),
803+
add_js_suffix_to_imports("import { Comp } from './Comp.tsx';"),
805804
"import { Comp } from './Comp.tsx';"
806805
);
807806
assert_eq!(
808-
replace_ts_imports_with_js("import { Button } from './Button.jsx';"),
807+
add_js_suffix_to_imports("import { Button } from './Button.jsx';"),
809808
"import { Button } from './Button.jsx';"
810809
);
811810

812811
// Should ignore absolute paths or URLs
813812
assert_eq!(
814-
replace_ts_imports_with_js("import React from 'react';"),
813+
add_js_suffix_to_imports("import React from 'react';"),
815814
"import React from 'react';"
816815
);
817816
assert_eq!(
818-
replace_ts_imports_with_js("import { BamlClient } from '@boundaryml/baml';"),
817+
add_js_suffix_to_imports("import { BamlClient } from '@boundaryml/baml';"),
819818
"import { BamlClient } from '@boundaryml/baml';"
820819
);
821820
assert_eq!(
822-
replace_ts_imports_with_js("const path = '/path/to/file.ts';"),
821+
add_js_suffix_to_imports("const path = '/path/to/file.ts';"),
823822
"const path = '/path/to/file.ts';" // This is not an import/export statement
824823
);
825824

826825
// Empty string
827-
assert_eq!(replace_ts_imports_with_js(""), "");
826+
assert_eq!(add_js_suffix_to_imports(""), "");
828827
// String with no imports
829828
assert_eq!(
830-
replace_ts_imports_with_js("const x = 10; function y() {}"),
829+
add_js_suffix_to_imports("const x = 10; function y() {}"),
831830
"const x = 10; function y() {}"
832831
);
833832
// Mixed content
834833
assert_eq!(
835-
replace_ts_imports_with_js(
834+
add_js_suffix_to_imports(
836835
"console.log('hello');\nimport { a } from './a.ts';\nimport { b } from './b';\nimport { c } from './c.js';\nimport { d } from 'd-lib';\nexport { e } from '../e.ts';\nconsole.log('world');"
837836
),
838837
"console.log('hello');\nimport { a } from './a.js';\nimport { b } from './b.js';\nimport { c } from './c.js';\nimport { d } from 'd-lib';\nexport { e } from '../e.js';\nconsole.log('world');"

‎integ-tests/baml_src/test-files/providers/openrouter.baml‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,8 @@ function TestOpenRouterMistralSmall3_1_24b(input: string) -> string {
44
Write a nice short story about {{ input }}. Keep it to 15 words or less.
55
"#
66
}
7-
7+
8+
89
test TestName {
910
functions [TestOpenRouterMistralSmall3_1_24b]
1011
args {

0 commit comments

Comments
 (0)