[feat] Improving Crashlytics MCP Tools #9181

maxl0rd · 2025-09-24T20:52:07Z

Description

Implementing batch get events tool and related prompts
Improving error handling
Hardening behavior against common errors

gemini-code-assist · 2025-09-24T20:52:29Z

Summary of Changes

Hello @maxl0rd, I'm Gemini Code Assist¹! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed!

This pull request significantly enhances the Crashlytics MCP tools by introducing a new batchGetEvents capability, which allows for efficient retrieval of multiple events. It also streamlines error handling across various Crashlytics API interactions by centralizing error management and removes redundant try-catch blocks. Furthermore, the PR adds robust validation for event filters to prevent common input errors and simplifies the structure of generated reports for better clarity and usability.

Highlights

New Batch Get Events Tool: Implemented a new batchGetEvents tool for Crashlytics, allowing retrieval of multiple events by their resource names, which is particularly useful for fetching sample crashes and exceptions.
Improved Error Handling: Refactored Crashlytics API calls by removing redundant try-catch blocks from individual API functions (listEvents, batchGetEvents, getIssue, updateIssue, deleteNote, listNotes, getReport). This suggests a more centralized or higher-level error handling mechanism, simplifying the code and making it more robust.
Enhanced Filter Validation: Introduced a new validateEventFilters function to enforce strict validation rules for event filters, including format checks for display names (e.g., 'manufacturer (device)') and a 90-day time limit for intervalStartTime. This hardens the behavior against common input errors.
Report Simplification: Added a simplifyReport function that prunes verbose fields (like model, manufacturer, buildVersion, displayVersion, os) from report groups, retaining only the displayName. This makes the report output more concise and easier to consume.
Updated MCP Tool Prompts: Modified the prompts for Crashlytics tools to guide users on the new batch_get_events tool and provide clearer instructions on how to use various report tools effectively, including suggestions for chaining tools.

Using Gemini Code Assist

The full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips.

Invoking Gemini

You can request assistance from Gemini at any point by creating a comment using either /gemini <command> or @gemini-code-assist <command>. Below is a summary of the supported commands on the current page.

Feature	Command	Description
Code Review	`/gemini review`	Performs a code review for the current pull request in its current state.
Pull Request Summary	`/gemini summary`	Provides a summary of the current pull request in its current state.
Comment	@gemini-code-assist	Responds in comments when explicitly tagged, both in pull request comments and review comments.
Help	`/gemini help`	Displays a list of available commands.

Customization

To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a .gemini/ folder in the base of the repository. Detailed instructions can be found here.

Limitations & Feedback

Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here.

You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension.

Review the Privacy Notices, Generative AI Prohibited Use Policy, Terms of Service, and learn how to configure Gemini Code Assist in GitHub here. Gemini can make mistakes, so double check it and use code with caution. ↩

gemini-code-assist

Code Review

This pull request introduces several improvements to the Crashlytics MCP tools, including adding a batchGetEvents tool, enhancing error handling by removing redundant try-catch blocks, and hardening the tools against common errors with better validation and data simplification. The changes are well-structured and improve the robustness of the tools. I've identified a few areas for improvement, mainly concerning object mutation which can lead to side effects, and some minor inconsistencies that could be addressed to improve maintainability and clarity.

src/crashlytics/reports.ts

src/crashlytics/events.ts

src/mcp/prompts/crashlytics/connect.ts

src/crashlytics/events.ts

src/crashlytics/filters.ts

schnecle

A couple small comments, otherwise LGTM.

schnecle · 2025-09-25T19:34:56Z

src/crashlytics/filters.ts

      .optional()
      .describe(
-        `Count FATAL events (crashes), NON_FATAL events (exceptions) or ANR events (application not responding)`,
+        `Counts FATAL events (crashes), NON_FATAL events (exceptions) or ANR events (application not responding)`,


Should we include a default value here? It seems like this is not being set by the LLM and maybe we should do the inverse and make the LLM add others if it wants them.

We haven't fully figured out when is the right time to guide it to fatals yet. Let's crack that off as a separate problem.

schnecle · 2025-09-25T19:36:47Z

src/crashlytics/reports.ts

+  const simplifiedReport = cloneDeep(report);
+  if (!simplifiedReport.groups) return report;
+  simplifiedReport.groups.forEach((group) => {
+    // Leaves displayName only in each group, which is the appropriate field to use


nit: can you describe what is included in the displayName in this comment?

schnecle · 2025-09-25T19:37:59Z

src/mcp/prompts/crashlytics/connect.ts

-  3. Use the 'crashlytics_list_events' tool to get an example crash for this issue.
-    3a. Apply the same filtering criteria that you used to find the issue, so that you find an appropriate event.
+  3. Use the 'crashlytics_batch_get_events' tool to get an example crash for this issue. Use the event names in the sampleEvent fields.
+    3a. If you need to read more events, use the 'crashlytics_list_events' tool.


Have you seen it be able to effectively use this?

Yes, it's reliably using the get tool with the provided sample events. This is pulling much more accurate samples now than the list path.

visumickey

My major concern is about "How are we communicating to the LLM about the failure of MCP tool calls?". Do we have a broader way to handle that better?

visumickey · 2025-09-25T19:35:58Z

src/crashlytics/filters.ts

+ */
+export function validateEventFilters(filter: EventFilter): void {
+  if (!filter) return;
+  const ninetyDaysAgo = new Date(Date.now() - 90 * 24 * 60 * 60 * 1000);


Can we utilize a constant for a day. That way it is easier to understand and resuable.

src/crashlytics/reports.ts

raymondlam

lgtm, just a few comments on the test coverage

raymondlam · 2025-09-25T19:32:30Z

src/crashlytics/events.spec.ts

      expect(nock.isDone()).to.be.true;
    });

-    it("should throw a FirebaseError if the API call fails", async () => {


Can we add tests to show how errors are handled now? Similarly for the other tools as well.

raymondlam · 2025-09-25T19:54:48Z

src/mcp/tools/crashlytics/reports.ts

+      additionalPrompt = "This report response contains no results.";
+    }
+    if (additionalPrompt) {
+      reportResponse.usage = (reportResponse.usage || "").concat("\n", additionalPrompt);


How does changing the usage field compare to just removing the extra usage field altogether.

raymondlam · 2025-09-25T19:55:38Z

src/crashlytics/events.ts

+    queryParams: queryParams,
+    timeout: TIMEOUT,
+  });
+  response.body.events ??= [];


nit: can you add a test to check that [] is returned.

maxl0rd · 2025-09-25T20:13:28Z

I removed many instances of "log and throw" in this PR. The general exception handling works well, both for ts errors and api response errors. There's no need to wrap every exception in every tool call. Removing this is better allowing the model to see the status code and exception message from the service, which is helping in many cases.

visumickey · 2025-09-25T22:53:57Z

I removed many instances of "log and throw" in this PR. The general exception handling works well, both for ts errors and api response errors. There's no need to wrap every exception in every tool call. Removing this is better allowing the model to see the status code and exception message from the service, which is helping in many cases.

Have you pushed the changes? I don't see any new commits.

visumickey · 2025-09-25T23:00:01Z

My major concern is about "How are we communicating to the LLM about the failure of MCP tool calls?". Do we have a broader way to handle that better?

Going to call this resolved since the goal is to communicate the response directly from the LLM. So, all good here.

github-project-automation bot added this to [Cloud] Extensions + Functions Sep 24, 2025

gemini-code-assist bot reviewed Sep 24, 2025

View reviewed changes

src/crashlytics/reports.ts Show resolved Hide resolved

src/crashlytics/events.ts Show resolved Hide resolved

src/crashlytics/events.ts Show resolved Hide resolved

src/mcp/prompts/crashlytics/connect.ts Outdated Show resolved Hide resolved

maxl0rd force-pushed the ml_sample_events branch from bad75a8 to 87e25c9 Compare September 25, 2025 18:15

visumickey reviewed Sep 25, 2025

View reviewed changes

src/crashlytics/events.ts Show resolved Hide resolved

src/crashlytics/filters.ts Show resolved Hide resolved

schnecle approved these changes Sep 25, 2025

View reviewed changes

github-project-automation bot moved this to Approved [PR] in [Cloud] Extensions + Functions Sep 25, 2025

visumickey reviewed Sep 25, 2025

View reviewed changes

raymondlam approved these changes Sep 25, 2025

View reviewed changes

visumickey approved these changes Sep 25, 2025

View reviewed changes

maxl0rd added 4 commits September 26, 2025 10:18

Implementing batch get events tool and related prompts

4bce225

Improving error handling in crashlytics tools

0b91866

Hardening crashlytics tools against common errors

55a3b66

Address code review comments

adc8626

maxl0rd force-pushed the ml_sample_events branch from 87e25c9 to adc8626 Compare September 26, 2025 14:18

maxl0rd merged commit 5270d3c into master Sep 26, 2025
48 checks passed

maxl0rd deleted the ml_sample_events branch September 26, 2025 15:31

github-project-automation bot moved this from Approved [PR] to Done in [Cloud] Extensions + Functions Sep 26, 2025

[feat] Improving Crashlytics MCP Tools #9181

[feat] Improving Crashlytics MCP Tools #9181

Uh oh!

Conversation

maxl0rd commented Sep 24, 2025

Description

Uh oh!

gemini-code-assist bot commented Sep 24, 2025

Summary of Changes

Highlights

Footnotes

Uh oh!

gemini-code-assist bot left a comment

Choose a reason for hiding this comment

Code Review

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

schnecle left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

visumickey left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

raymondlam left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

maxl0rd commented Sep 25, 2025

Uh oh!

visumickey commented Sep 25, 2025

Uh oh!

visumickey commented Sep 25, 2025

Uh oh!

Uh oh!

Uh oh!