Skip to content
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.

Sign up for GitHub

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

Add X-Firebase-AppId header to VertexAI requests #8809

Open
wants to merge 4 commits into
base: main
Choose a base branch
from
Open

Add X-Firebase-AppId header to VertexAI requests #8809

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
1df52ad
Add X-Firebase-AppId header to requests
dlarocque Feb 25, 2025
7c62bc1
Add changeset
dlarocque Feb 25, 2025
7f14785
Run formatter
dlarocque Feb 25, 2025
767cd77
Only send if data collection is on
dlarocque Feb 25, 2025
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
5 changes: 5 additions & 0 deletions .changeset/red-hornets-peel.md
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
'@firebase/vertexai': patch
---

Throw an error when initializing models if `appId` is not defined in the given `VertexAI` instance.
1 change: 1 addition & 0 deletions common/api-review/vertexai.api.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -817,6 +817,7 @@ export const enum VertexAIErrorCode {
INVALID_CONTENT = "invalid-content",
INVALID_SCHEMA = "invalid-schema",
NO_API_KEY = "no-api-key",
NO_APP_ID = "no-app-id",
NO_MODEL = "no-model",
NO_PROJECT_ID = "no-project-id",
PARSE_FAILED = "parse-failed",
Expand Down
1 change: 1 addition & 0 deletions docs-devsite/vertexai.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -545,6 +545,7 @@ export declare const enum VertexAIErrorCode
| INVALID\_CONTENT | <code>&quot;invalid-content&quot;</code> | An error associated with a Content object. |
| INVALID\_SCHEMA | <code>&quot;invalid-schema&quot;</code> | An error due to invalid Schema input. |
| NO\_API\_KEY | <code>&quot;no-api-key&quot;</code> | An error occurred due to a missing Firebase API key. |
| NO\_APP\_ID | <code>&quot;no-app-id&quot;</code> | An error occured due to a missing app ID. |
| NO\_MODEL | <code>&quot;no-model&quot;</code> | An error occurred due to a model name not being specified during initialization. |
| NO\_PROJECT\_ID | <code>&quot;no-project-id&quot;</code> | An error occurred due to a missing project ID. |
| PARSE\_FAILED | <code>&quot;parse-failed&quot;</code> | An error occurred while parsing. |
Expand Down
43 changes: 38 additions & 5 deletions packages/vertexai/src/api.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,8 @@ const fakeVertexAI: VertexAI = {
automaticDataCollectionEnabled: true,
options: {
apiKey: 'key',
projectId: 'my-project'
projectId: 'my-project',
appId: 'my-appid'
}
},
location: 'us-central1'
Expand All @@ -48,7 +49,7 @@ describe('Top level API', () => {
it('getGenerativeModel throws if no apiKey is provided', () => {
const fakeVertexNoApiKey = {
...fakeVertexAI,
app: { options: { projectId: 'my-project' } }
app: { options: { projectId: 'my-project', appId: 'my-appid' } }
} as VertexAI;
try {
getGenerativeModel(fakeVertexNoApiKey, { model: 'my-model' });
Expand All @@ -64,7 +65,7 @@ describe('Top level API', () => {
it('getGenerativeModel throws if no projectId is provided', () => {
const fakeVertexNoProject = {
...fakeVertexAI,
app: { options: { apiKey: 'my-key' } }
app: { options: { apiKey: 'my-key', appId: 'my-appid' } }
} as VertexAI;
try {
getGenerativeModel(fakeVertexNoProject, { model: 'my-model' });
Expand All @@ -79,6 +80,22 @@ describe('Top level API', () => {
);
}
});
it('getGenerativeModel throws if no appId is provided', () => {
const fakeVertexNoProject = {
...fakeVertexAI,
app: { options: { apiKey: 'my-key', projectId: 'my-projectid' } }
} as VertexAI;
try {
getGenerativeModel(fakeVertexNoProject, { model: 'my-model' });
} catch (e) {
expect((e as VertexAIError).code).includes(VertexAIErrorCode.NO_APP_ID);
expect((e as VertexAIError).message).equals(
`VertexAI: The "appId" field is empty in the local` +
` Firebase config. Firebase VertexAI requires this field ` +
`to contain a valid app ID. (vertexAI/${VertexAIErrorCode.NO_APP_ID})`
);
}
});
it('getGenerativeModel gets a GenerativeModel', () => {
const genModel = getGenerativeModel(fakeVertexAI, { model: 'my-model' });
expect(genModel).to.be.an.instanceOf(GenerativeModel);
Expand All @@ -98,7 +115,7 @@ describe('Top level API', () => {
it('getImagenModel throws if no apiKey is provided', () => {
const fakeVertexNoApiKey = {
...fakeVertexAI,
app: { options: { projectId: 'my-project' } }
app: { options: { projectId: 'my-project', appId: 'my-appid' } }
} as VertexAI;
try {
getImagenModel(fakeVertexNoApiKey, { model: 'my-model' });
Expand All @@ -114,7 +131,7 @@ describe('Top level API', () => {
it('getImagenModel throws if no projectId is provided', () => {
const fakeVertexNoProject = {
...fakeVertexAI,
app: { options: { apiKey: 'my-key' } }
app: { options: { apiKey: 'my-key', appId: 'my-appid' } }
} as VertexAI;
try {
getImagenModel(fakeVertexNoProject, { model: 'my-model' });
Expand All @@ -129,6 +146,22 @@ describe('Top level API', () => {
);
}
});
it('getImagenModel throws if no appId is provided', () => {
const fakeVertexNoProject = {
...fakeVertexAI,
app: { options: { apiKey: 'my-key', projectId: 'my-project' } }
} as VertexAI;
try {
getImagenModel(fakeVertexNoProject, { model: 'my-model' });
} catch (e) {
expect((e as VertexAIError).code).includes(VertexAIErrorCode.NO_APP_ID);
expect((e as VertexAIError).message).equals(
`VertexAI: The "appId" field is empty in the local` +
` Firebase config. Firebase VertexAI requires this field ` +
`to contain a valid app ID. (vertexAI/${VertexAIErrorCode.NO_APP_ID})`
);
}
});
it('getImagenModel gets an ImagenModel', () => {
const genModel = getImagenModel(fakeVertexAI, { model: 'my-model' });
expect(genModel).to.be.an.instanceOf(ImagenModel);
Expand Down
1 change: 1 addition & 0 deletions packages/vertexai/src/methods/chat-session.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,7 @@ use(chaiAsPromised);
const fakeApiSettings: ApiSettings = {
apiKey: 'key',
project: 'my-project',
appId: 'my-appid',
location: 'us-central1'
};

Expand Down
1 change: 1 addition & 0 deletions packages/vertexai/src/methods/count-tokens.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,7 @@ use(chaiAsPromised);
const fakeApiSettings: ApiSettings = {
apiKey: 'key',
project: 'my-project',
appId: 'my-appid',
location: 'us-central1'
};

Expand Down
1 change: 1 addition & 0 deletions packages/vertexai/src/methods/generate-content.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,7 @@ use(chaiAsPromised);
const fakeApiSettings: ApiSettings = {
apiKey: 'key',
project: 'my-project',
appId: 'my-appid',
location: 'us-central1'
};

Expand Down
3 changes: 2 additions & 1 deletion packages/vertexai/src/models/generative-model.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,8 @@ const fakeVertexAI: VertexAI = {
automaticDataCollectionEnabled: true,
options: {
apiKey: 'key',
projectId: 'my-project'
projectId: 'my-project',
appId: 'my-appid'
}
},
location: 'us-central1'
Expand Down
3 changes: 2 additions & 1 deletion packages/vertexai/src/models/imagen-model.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,8 @@ const fakeVertexAI: VertexAI = {
automaticDataCollectionEnabled: true,
options: {
apiKey: 'key',
projectId: 'my-project'
projectId: 'my-project',
appId: 'my-appid'
}
},
location: 'us-central1'
Expand Down
21 changes: 20 additions & 1 deletion packages/vertexai/src/models/vertexai-model.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,8 @@ const fakeVertexAI: VertexAI = {
automaticDataCollectionEnabled: true,
options: {
apiKey: 'key',
projectId: 'my-project'
projectId: 'my-project',
appId: 'my-appid'
}
},
location: 'us-central1'
Expand Down Expand Up @@ -100,4 +101,22 @@ describe('VertexAIModel', () => {
);
}
});
it('throws if not passed an app ID', () => {
const fakeVertexAI: VertexAI = {
app: {
name: 'DEFAULT',
automaticDataCollectionEnabled: true,
options: {
apiKey: 'key',
projectId: 'my-project'
}
},
location: 'us-central1'
};
try {
new TestModel(fakeVertexAI, 'my-model');
} catch (e) {
expect((e as VertexAIError).code).to.equal(VertexAIErrorCode.NO_APP_ID);
}
});
});
8 changes: 8 additions & 0 deletions packages/vertexai/src/models/vertexai-model.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -68,10 +68,18 @@ export abstract class VertexAIModel {
VertexAIErrorCode.NO_PROJECT_ID,
`The "projectId" field is empty in the local Firebase config. Firebase VertexAI requires this field to contain a valid project ID.`
);
} else if (!vertexAI.app?.options?.appId) {
throw new VertexAIError(
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In light of Paul's comment about automaticDataCollectionEnabled, not sure if we throw this error if that's false, since we don't actually need appId in that case? Kind of weird edge case though.

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The firebaseAppConfig obtained from Firebase Console and Terraform always contains an appId, so unless the developer deliberately removes the appId (not sure why since it's not sensitive), it should always be present. AppId is not just for telemetry purposes. Thus, stronger validation is better here.

VertexAIErrorCode.NO_APP_ID,
`The "appId" field is empty in the local Firebase config. Firebase VertexAI requires this field to contain a valid app ID.`
);
} else {
this._apiSettings = {
apiKey: vertexAI.app.options.apiKey,
project: vertexAI.app.options.projectId,
appId: vertexAI.app.options.appId,
automaticDataCollectionEnabled:
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No action needed, just wondering out loud if at some number of properties, we just pass vertexAI.app or at least vertexAI.app.options through instead of passing them through one by one.

vertexAI.app.automaticDataCollectionEnabled,
location: vertexAI.location
};

Expand Down
49 changes: 49 additions & 0 deletions packages/vertexai/src/requests/request.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,7 @@ use(chaiAsPromised);
const fakeApiSettings: ApiSettings = {
apiKey: 'key',
project: 'my-project',
appId: 'my-appid',
location: 'us-central1'
};

Expand Down Expand Up @@ -103,6 +104,7 @@ describe('request methods', () => {
const fakeApiSettings: ApiSettings = {
apiKey: 'key',
project: 'myproject',
appId: 'my-appid',
location: 'moon',
getAuthToken: () => Promise.resolve({ accessToken: 'authtoken' }),
getAppCheckToken: () => Promise.resolve({ token: 'appchecktoken' })
Expand All @@ -124,6 +126,50 @@ describe('request methods', () => {
const headers = await getHeaders(fakeUrl);
expect(headers.get('x-goog-api-key')).to.equal('key');
});
it('adds app id if automatedDataCollectionEnabled is undefined', async () => {
const headers = await getHeaders(fakeUrl);
expect(headers.get('X-Firebase-AppId')).to.equal('my-appid');
});
it('adds app id if automatedDataCollectionEnabled is true', async () => {
const fakeApiSettings: ApiSettings = {
apiKey: 'key',
project: 'myproject',
appId: 'my-appid',
location: 'moon',
automaticDataCollectionEnabled: true,
getAuthToken: () => Promise.resolve({ accessToken: 'authtoken' }),
getAppCheckToken: () => Promise.resolve({ token: 'appchecktoken' })
};
const fakeUrl = new RequestUrl(
'models/model-name',
Task.GENERATE_CONTENT,
fakeApiSettings,
true,
{}
);
const headers = await getHeaders(fakeUrl);
expect(headers.get('X-Firebase-AppId')).to.equal('my-appid');
});
it('does not add app id if automatedDataCollectionEnabled is false', async () => {
const fakeApiSettings: ApiSettings = {
apiKey: 'key',
project: 'myproject',
appId: 'my-appid',
location: 'moon',
automaticDataCollectionEnabled: false,
getAuthToken: () => Promise.resolve({ accessToken: 'authtoken' }),
getAppCheckToken: () => Promise.resolve({ token: 'appchecktoken' })
};
const fakeUrl = new RequestUrl(
'models/model-name',
Task.GENERATE_CONTENT,
fakeApiSettings,
true,
{}
);
const headers = await getHeaders(fakeUrl);
expect(headers.get('X-Firebase-AppId')).to.be.null;
});
it('adds app check token if it exists', async () => {
const headers = await getHeaders(fakeUrl);
expect(headers.get('X-Firebase-AppCheck')).to.equal('appchecktoken');
Expand All @@ -135,6 +181,7 @@ describe('request methods', () => {
{
apiKey: 'key',
project: 'myproject',
appId: 'my-appid',
location: 'moon'
},
true,
Expand Down Expand Up @@ -167,6 +214,7 @@ describe('request methods', () => {
{
apiKey: 'key',
project: 'myproject',
appId: 'my-appid',
location: 'moon',
getAppCheckToken: () =>
Promise.resolve({ token: 'dummytoken', error: Error('oops') })
Expand All @@ -193,6 +241,7 @@ describe('request methods', () => {
{
apiKey: 'key',
project: 'myproject',
appId: 'my-appid',
location: 'moon'
},
true,
Expand Down
3 changes: 3 additions & 0 deletions packages/vertexai/src/requests/request.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -84,6 +84,9 @@ export async function getHeaders(url: RequestUrl): Promise<Headers> {
headers.append('Content-Type', 'application/json');
headers.append('x-goog-api-client', getClientHeaders());
headers.append('x-goog-api-key', url.apiSettings.apiKey);
if (url.apiSettings.automaticDataCollectionEnabled !== false) {
Copy link
Contributor

@hsubox76 hsubox76 Feb 27, 2025 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe we should be safe and make this fail on any falsy value in case, say, the user sets it to undefined for some reason. Sometimes that's how people unset properties. So just if (url.apiSettings.automaticDataCollectionEnabled)? I do think this is a very edge case, so consider it a nit.

headers.append('X-Firebase-AppId', url.apiSettings.appId);
}
if (url.apiSettings.getAppCheckToken) {
const appCheckToken = await url.apiSettings.getAppCheckToken();
if (appCheckToken) {
Expand Down
3 changes: 3 additions & 0 deletions packages/vertexai/src/types/error.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -87,6 +87,9 @@ export const enum VertexAIErrorCode {
/** An error occurred due to a missing Firebase API key. */
NO_API_KEY = 'no-api-key',

/** An error occured due to a missing app ID. */
NO_APP_ID = 'no-app-id',

/** An error occurred due to a model name not being specified during initialization. */
NO_MODEL = 'no-model',

Expand Down
2 changes: 2 additions & 0 deletions packages/vertexai/src/types/internal.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,9 @@ export * from './imagen/internal';
export interface ApiSettings {
apiKey: string;
project: string;
appId: string;
location: string;
automaticDataCollectionEnabled?: boolean;
getAuthToken?: () => Promise<FirebaseAuthTokenData | null>;
getAppCheckToken?: () => Promise<AppCheckTokenResult>;
}
Loading