Skip to content

Commit 5270d3c

Browse files
authored
[feat] Improving Crashlytics MCP Tools (#9181)
* Implementing batch get events tool and related prompts * Improving error handling in crashlytics tools * Hardening crashlytics tools against common errors
1 parent 0fc7a29 commit 5270d3c

File tree

15 files changed

+563
-281
lines changed

15 files changed

+563
-281
lines changed

src/crashlytics/events.spec.ts

Lines changed: 1 addition & 29 deletions
Original file line numberDiff line numberDiff line change
@@ -60,20 +60,6 @@ describe("events", () => {
6060
expect(nock.isDone()).to.be.true;
6161
});
6262

63-
it("should throw a FirebaseError if the API call fails", async () => {
64-
nock(crashlyticsApiOrigin())
65-
.get(`/v1alpha/projects/${requestProjectNumber}/apps/${appId}/events`)
66-
.query({
67-
page_size: String(pageSize),
68-
})
69-
.reply(500, { error: "Internal Server Error" });
70-
71-
await expect(listEvents(appId, {}, pageSize)).to.be.rejectedWith(
72-
FirebaseError,
73-
`Failed to list events for app_id ${appId}.`,
74-
);
75-
});
76-
7763
it("should throw a FirebaseError if the appId is invalid", async () => {
7864
const invalidAppId = "invalid-app-id";
7965

@@ -97,7 +83,7 @@ describe("events", () => {
9783
nock(crashlyticsApiOrigin())
9884
.get(`/v1alpha/projects/${requestProjectNumber}/apps/${appId}/events:batchGet`)
9985
.query({
100-
"event.names": eventNames,
86+
names: eventNames,
10187
})
10288
.reply(200, mockResponse);
10389

@@ -107,20 +93,6 @@ describe("events", () => {
10793
expect(nock.isDone()).to.be.true;
10894
});
10995

110-
it("should throw a FirebaseError if the API call fails", async () => {
111-
nock(crashlyticsApiOrigin())
112-
.get(`/v1alpha/projects/${requestProjectNumber}/apps/${appId}/events:batchGet`)
113-
.query({
114-
"event.names": eventNames,
115-
})
116-
.reply(500, { error: "Internal Server Error" });
117-
118-
await expect(batchGetEvents(appId, eventNames)).to.be.rejectedWith(
119-
FirebaseError,
120-
`Failed to batch get events for app_id ${appId}.`,
121-
);
122-
});
123-
12496
it("should throw a FirebaseError if there are too many events", async () => {
12597
const tooManyEventNames = Array.from(Array(101).keys()).map(
12698
(i) =>

src/crashlytics/events.ts

Lines changed: 29 additions & 45 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
import { logger } from "../logger";
2-
import { FirebaseError, getError } from "../error";
2+
import { FirebaseError } from "../error";
33
import { CRASHLYTICS_API_CLIENT, parseProjectNumber, TIMEOUT } from "./utils";
44
import { BatchGetEventsResponse, ListEventsResponse } from "./types";
55
import { EventFilter, filterToUrlSearchParams } from "./filters";
@@ -17,31 +17,22 @@ export async function listEvents(
1717
pageSize = 1,
1818
): Promise<ListEventsResponse> {
1919
const requestProjectNumber = parseProjectNumber(appId);
20-
21-
try {
22-
const queryParams = filterToUrlSearchParams(filter);
23-
queryParams.set("page_size", `${pageSize}`);
24-
25-
logger.debug(
26-
`[crashlytics] listEvents called with appId: ${appId}, filter: ${queryParams.toString()}, pageSize: ${pageSize}`,
27-
);
28-
29-
const response = await CRASHLYTICS_API_CLIENT.request<void, ListEventsResponse>({
30-
method: "GET",
31-
headers: {
32-
"Content-Type": "application/json",
33-
},
34-
path: `/projects/${requestProjectNumber}/apps/${appId}/events`,
35-
queryParams: queryParams,
36-
timeout: TIMEOUT,
37-
});
38-
39-
return response.body;
40-
} catch (err: unknown) {
41-
throw new FirebaseError(`Failed to list events for app_id ${appId}.`, {
42-
original: getError(err),
43-
});
44-
}
20+
const queryParams = filterToUrlSearchParams(filter);
21+
queryParams.set("page_size", `${pageSize}`);
22+
logger.debug(
23+
`[crashlytics] listEvents called with appId: ${appId}, filter: ${queryParams.toString()}, pageSize: ${pageSize}`,
24+
);
25+
const response = await CRASHLYTICS_API_CLIENT.request<void, ListEventsResponse>({
26+
method: "GET",
27+
headers: {
28+
"Content-Type": "application/json",
29+
},
30+
path: `/projects/${requestProjectNumber}/apps/${appId}/events`,
31+
queryParams: queryParams,
32+
timeout: TIMEOUT,
33+
});
34+
response.body.events ??= [];
35+
return response.body;
4536
}
4637

4738
/**
@@ -63,24 +54,17 @@ export async function batchGetEvents(
6354
);
6455
const queryParams = new URLSearchParams();
6556
eventNames.forEach((en) => {
66-
queryParams.append("event.names", en);
57+
queryParams.append("names", en);
6758
});
68-
69-
try {
70-
const response = await CRASHLYTICS_API_CLIENT.request<void, BatchGetEventsResponse>({
71-
method: "GET",
72-
headers: {
73-
"Content-Type": "application/json",
74-
},
75-
path: `/projects/${requestProjectNumber}/apps/${appId}/events:batchGet`,
76-
queryParams: queryParams,
77-
timeout: TIMEOUT,
78-
});
79-
80-
return response.body;
81-
} catch (err: unknown) {
82-
throw new FirebaseError(`Failed to batch get events for app_id ${appId}.`, {
83-
original: getError(err),
84-
});
85-
}
59+
const response = await CRASHLYTICS_API_CLIENT.request<void, BatchGetEventsResponse>({
60+
method: "GET",
61+
headers: {
62+
"Content-Type": "application/json",
63+
},
64+
path: `/projects/${requestProjectNumber}/apps/${appId}/events:batchGet`,
65+
queryParams: queryParams,
66+
timeout: TIMEOUT,
67+
});
68+
response.body.events ??= [];
69+
return response.body;
8670
}

src/crashlytics/filters.spec.ts

Lines changed: 177 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,177 @@
1+
import * as chai from "chai";
2+
import * as chaiAsPromised from "chai-as-promised";
3+
4+
import { validateEventFilters, EventFilter } from "./filters";
5+
import { FirebaseError } from "../error";
6+
7+
chai.use(chaiAsPromised);
8+
const expect = chai.expect;
9+
10+
describe("filters", () => {
11+
describe("validateEventFilters", () => {
12+
it("should not throw for undefined filter", () => {
13+
expect(() => validateEventFilters(undefined)).to.not.throw();
14+
});
15+
16+
it("should not throw for empty filter", () => {
17+
expect(() => validateEventFilters({} as EventFilter)).to.not.throw();
18+
});
19+
20+
describe("deviceDisplayNames validation", () => {
21+
it("should not throw for valid device display name format", () => {
22+
const filter: EventFilter = {
23+
deviceDisplayNames: ["Samsung (Galaxy S21)", "Google (Pixel 6)", "Apple (iPhone 13)"],
24+
};
25+
expect(() => validateEventFilters(filter)).to.not.throw();
26+
});
27+
28+
it("should throw for invalid device display name without parentheses", () => {
29+
const filter: EventFilter = {
30+
deviceDisplayNames: ["Samsung Galaxy S21"],
31+
};
32+
expect(() => validateEventFilters(filter)).to.throw(
33+
FirebaseError,
34+
"deviceDisplayNames must match pattern 'manufacturer (device)'",
35+
);
36+
});
37+
38+
it("should throw for invalid device display name with missing manufacturer", () => {
39+
const filter: EventFilter = {
40+
deviceDisplayNames: ["(Galaxy S21)"],
41+
};
42+
expect(() => validateEventFilters(filter)).to.throw(
43+
FirebaseError,
44+
"deviceDisplayNames must match pattern 'manufacturer (device)'",
45+
);
46+
});
47+
48+
it("should throw for invalid device display name with empty parentheses", () => {
49+
const filter: EventFilter = {
50+
deviceDisplayNames: ["Samsung ()"],
51+
};
52+
expect(() => validateEventFilters(filter)).to.throw(
53+
FirebaseError,
54+
"deviceDisplayNames must match pattern 'manufacturer (device)'",
55+
);
56+
});
57+
58+
it("should throw when any device display name is invalid", () => {
59+
const filter: EventFilter = {
60+
deviceDisplayNames: ["Samsung (Galaxy S21)", "InvalidFormat", "Google (Pixel 6)"],
61+
};
62+
expect(() => validateEventFilters(filter)).to.throw(
63+
FirebaseError,
64+
"deviceDisplayNames must match pattern 'manufacturer (device)'",
65+
);
66+
});
67+
});
68+
69+
describe("operatingSystemDisplayNames validation", () => {
70+
it("should not throw for valid OS display name format", () => {
71+
const filter: EventFilter = {
72+
operatingSystemDisplayNames: ["iOS (15.0)", "Android (12)", "Windows (11)"],
73+
};
74+
expect(() => validateEventFilters(filter)).to.not.throw();
75+
});
76+
77+
it("should throw for invalid OS display name without parentheses", () => {
78+
const filter: EventFilter = {
79+
operatingSystemDisplayNames: ["iOS 15.0"],
80+
};
81+
expect(() => validateEventFilters(filter)).to.throw(
82+
FirebaseError,
83+
"operatingSystemDisplayNames must match pattern 'os (version)'",
84+
);
85+
});
86+
87+
it("should throw for invalid OS display name with empty parentheses", () => {
88+
const filter: EventFilter = {
89+
operatingSystemDisplayNames: ["iOS ()"],
90+
};
91+
expect(() => validateEventFilters(filter)).to.throw(
92+
FirebaseError,
93+
"operatingSystemDisplayNames must match pattern 'os (version)'",
94+
);
95+
});
96+
});
97+
98+
describe("versionDisplayNames validation", () => {
99+
it("should not throw for valid version display name format", () => {
100+
const filter: EventFilter = {
101+
versionDisplayNames: ["1.0.0 (100)", "2.1.3 (213)", "3.0.0-beta (300)"],
102+
};
103+
expect(() => validateEventFilters(filter)).to.not.throw();
104+
});
105+
106+
it("should throw for invalid version display name without parentheses", () => {
107+
const filter: EventFilter = {
108+
versionDisplayNames: ["1.0.0 build 100"],
109+
};
110+
expect(() => validateEventFilters(filter)).to.throw(
111+
FirebaseError,
112+
"versionDisplayNames must match pattern 'version (build)'",
113+
);
114+
});
115+
116+
it("should throw for invalid version display name with missing version", () => {
117+
const filter: EventFilter = {
118+
versionDisplayNames: ["(100)"],
119+
};
120+
expect(() => validateEventFilters(filter)).to.throw(
121+
FirebaseError,
122+
"versionDisplayNames must match pattern 'version (build)'",
123+
);
124+
});
125+
126+
it("should throw when any version display name is invalid", () => {
127+
const filter: EventFilter = {
128+
versionDisplayNames: ["1.0.0 (100)", "InvalidFormat", "2.0.0 (200)"],
129+
};
130+
expect(() => validateEventFilters(filter)).to.throw(
131+
FirebaseError,
132+
"versionDisplayNames must match pattern 'version (build)'",
133+
);
134+
});
135+
});
136+
137+
describe("intervalStartTime validation", () => {
138+
it("should not throw for intervalStartTime within 90 days", () => {
139+
const thirtyDaysAgo = new Date(Date.now() - 30 * 24 * 60 * 60 * 1000).toISOString();
140+
const filter: EventFilter = {
141+
intervalStartTime: thirtyDaysAgo,
142+
};
143+
expect(() => validateEventFilters(filter)).to.not.throw();
144+
});
145+
146+
it("should not throw for intervalStartTime exactly 89 days ago", () => {
147+
const eightyNineDaysAgo = new Date(Date.now() - 89 * 24 * 60 * 60 * 1000).toISOString();
148+
const filter: EventFilter = {
149+
intervalStartTime: eightyNineDaysAgo,
150+
};
151+
expect(() => validateEventFilters(filter)).to.not.throw();
152+
});
153+
154+
it("should throw for intervalStartTime more than 90 days in the past", () => {
155+
const ninetyOneDaysAgo = new Date(Date.now() - 91 * 24 * 60 * 60 * 1000).toISOString();
156+
const filter: EventFilter = {
157+
intervalStartTime: ninetyOneDaysAgo,
158+
};
159+
expect(() => validateEventFilters(filter)).to.throw(
160+
FirebaseError,
161+
"intervalStartTime must be less than 90 days in the past",
162+
);
163+
});
164+
165+
it("should throw for intervalStartTime 100 days in the past", () => {
166+
const hundredDaysAgo = new Date(Date.now() - 100 * 24 * 60 * 60 * 1000).toISOString();
167+
const filter: EventFilter = {
168+
intervalStartTime: hundredDaysAgo,
169+
};
170+
expect(() => validateEventFilters(filter)).to.throw(
171+
FirebaseError,
172+
"intervalStartTime must be less than 90 days in the past",
173+
);
174+
});
175+
});
176+
});
177+
});

0 commit comments

Comments
 (0)