Skip to content

Commit 9077dfa

Browse files
RafaelPoclaude
andcommitted
Address code review: error handling, range rename, public API, token docs
- Wrap all 5 sheets tool functions in try/except for httpx.HTTPStatusError with user-friendly messages for 403/404/429 and generic Google API errors - Rename `range` parameter to `cell_range` in GoogleSheetsClient methods to avoid shadowing the Python builtin (model field stays `range` for API) - Use public `remove_tool()` API instead of `_tools.pop()` for stdio mode sheets tool removal in server.py - Document single-tenant assumption on Google token key in sheets_client.py Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
1 parent 825b5e8 commit 9077dfa

3 files changed

Lines changed: 95 additions & 60 deletions

File tree

everyrow-mcp/src/everyrow_mcp/server.py

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -124,7 +124,7 @@ def main():
124124
"sheets_create",
125125
"sheets_info",
126126
):
127-
mcp._tool_manager._tools.pop(name, None)
127+
mcp._tool_manager.remove_tool(name)
128128

129129
mcp.run(transport=transport.value)
130130

everyrow-mcp/src/everyrow_mcp/sheets_client.py

Lines changed: 8 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -41,7 +41,8 @@ async def get_google_token() -> str:
4141

4242
redis = get_redis_client()
4343

44-
# Try to get the stored token
44+
# NOTE: single-tenant — "current" key is shared. If multi-tenancy is
45+
# needed, key by session/user ID instead.
4546
token_key = build_key("google_token", "current")
4647
token_data = await redis.get(token_key)
4748
if token_data:
@@ -147,14 +148,14 @@ async def __aexit__(self, *args: Any) -> None:
147148
await self.close()
148149

149150
async def read_range(
150-
self, spreadsheet_id: str, range: str = "Sheet1"
151+
self, spreadsheet_id: str, cell_range: str = "Sheet1"
151152
) -> list[list[str]]:
152153
"""Read values from a spreadsheet range.
153154
154155
Returns a 2D list of strings (rows x columns).
155156
"""
156157
resp = await self._client.get(
157-
f"{SHEETS_API_BASE}/{spreadsheet_id}/values/{range}",
158+
f"{SHEETS_API_BASE}/{spreadsheet_id}/values/{cell_range}",
158159
params={"valueRenderOption": "FORMATTED_VALUE"},
159160
)
160161
resp.raise_for_status()
@@ -164,12 +165,12 @@ async def read_range(
164165
async def write_range(
165166
self,
166167
spreadsheet_id: str,
167-
range: str,
168+
cell_range: str,
168169
values: list[list[str]],
169170
) -> dict[str, Any]:
170171
"""Write values to a spreadsheet range (overwrite)."""
171172
resp = await self._client.put(
172-
f"{SHEETS_API_BASE}/{spreadsheet_id}/values/{range}",
173+
f"{SHEETS_API_BASE}/{spreadsheet_id}/values/{cell_range}",
173174
params={"valueInputOption": "USER_ENTERED"},
174175
json={"values": values},
175176
)
@@ -179,12 +180,12 @@ async def write_range(
179180
async def append_range(
180181
self,
181182
spreadsheet_id: str,
182-
range: str,
183+
cell_range: str,
183184
values: list[list[str]],
184185
) -> dict[str, Any]:
185186
"""Append values after existing data in a range."""
186187
resp = await self._client.post(
187-
f"{SHEETS_API_BASE}/{spreadsheet_id}/values/{range}:append",
188+
f"{SHEETS_API_BASE}/{spreadsheet_id}/values/{cell_range}:append",
188189
params={
189190
"valueInputOption": "USER_ENTERED",
190191
"insertDataOption": "INSERT_ROWS",

everyrow-mcp/src/everyrow_mcp/sheets_tools.py

Lines changed: 86 additions & 52 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,7 @@
99
import json
1010
import logging
1111

12+
import httpx
1213
from mcp.types import TextContent, ToolAnnotations
1314

1415
from everyrow_mcp.app import mcp
@@ -29,6 +30,20 @@
2930
logger = logging.getLogger(__name__)
3031

3132

33+
def _error_message(e: Exception) -> str:
34+
"""Format a user-friendly error message from a Google API exception."""
35+
if isinstance(e, httpx.HTTPStatusError):
36+
status = e.response.status_code
37+
if status == 403:
38+
return "Permission denied. Check that the spreadsheet is shared with you."
39+
if status == 404:
40+
return "Spreadsheet not found. Check the spreadsheet ID or URL."
41+
if status == 429:
42+
return "Rate limited by Google API. Please try again in a moment."
43+
return f"Google API error (HTTP {status}): {e.response.text}"
44+
return f"Error: {e!r}"
45+
46+
3247
@mcp.tool(
3348
name="sheets_list",
3449
annotations=ToolAnnotations(
@@ -41,11 +56,14 @@
4156
)
4257
async def sheets_list(params: SheetsListInput) -> list[TextContent]:
4358
"""List the user's Google Sheets, optionally filtered by name."""
44-
token = await get_google_token()
45-
async with GoogleSheetsClient(token) as client:
46-
files = await client.list_spreadsheets(
47-
query=params.query, max_results=params.max_results
48-
)
59+
try:
60+
token = await get_google_token()
61+
async with GoogleSheetsClient(token) as client:
62+
files = await client.list_spreadsheets(
63+
query=params.query, max_results=params.max_results
64+
)
65+
except Exception as e:
66+
return [TextContent(type="text", text=_error_message(e))]
4967

5068
if not files:
5169
msg = "No spreadsheets found"
@@ -83,9 +101,14 @@ async def sheets_read(params: SheetsReadInput) -> list[TextContent]:
83101
everyrow_agent(input_json=data, task="Research each company")
84102
sheets_write(spreadsheet_id="...", data=enriched_results)
85103
"""
86-
token = await get_google_token()
87-
async with GoogleSheetsClient(token) as client:
88-
values = await client.read_range(params.spreadsheet_id, params.range)
104+
try:
105+
token = await get_google_token()
106+
async with GoogleSheetsClient(token) as client:
107+
values = await client.read_range(
108+
params.spreadsheet_id, cell_range=params.range
109+
)
110+
except Exception as e:
111+
return [TextContent(type="text", text=_error_message(e))]
89112

90113
records = values_to_records(values)
91114

@@ -127,36 +150,41 @@ async def sheets_write(params: SheetsWriteInput) -> list[TextContent]:
127150
128151
Use append=True to add rows after existing data instead of overwriting.
129152
"""
130-
token = await get_google_token()
131-
values = records_to_values(params.data)
132-
133-
async with GoogleSheetsClient(token) as client:
134-
if params.append:
135-
result = await client.append_range(
136-
params.spreadsheet_id, params.range, values
137-
)
138-
updated_range = result.get("updates", {}).get("updatedRange", params.range)
139-
updated_rows = result.get("updates", {}).get(
140-
"updatedRows", len(params.data)
141-
)
142-
return [
143-
TextContent(
144-
type="text",
145-
text=f"Appended {updated_rows} rows to {updated_range}.",
153+
try:
154+
token = await get_google_token()
155+
values = records_to_values(params.data)
156+
157+
async with GoogleSheetsClient(token) as client:
158+
if params.append:
159+
result = await client.append_range(
160+
params.spreadsheet_id, cell_range=params.range, values=values
146161
)
147-
]
148-
else:
149-
result = await client.write_range(
150-
params.spreadsheet_id, params.range, values
151-
)
152-
updated_range = result.get("updatedRange", params.range)
153-
updated_rows = result.get("updatedRows", len(params.data) + 1)
154-
return [
155-
TextContent(
156-
type="text",
157-
text=f"Wrote {updated_rows} rows (including header) to {updated_range}.",
162+
updated_range = result.get("updates", {}).get(
163+
"updatedRange", params.range
164+
)
165+
updated_rows = result.get("updates", {}).get(
166+
"updatedRows", len(params.data)
158167
)
159-
]
168+
return [
169+
TextContent(
170+
type="text",
171+
text=f"Appended {updated_rows} rows to {updated_range}.",
172+
)
173+
]
174+
else:
175+
result = await client.write_range(
176+
params.spreadsheet_id, cell_range=params.range, values=values
177+
)
178+
updated_range = result.get("updatedRange", params.range)
179+
updated_rows = result.get("updatedRows", len(params.data) + 1)
180+
return [
181+
TextContent(
182+
type="text",
183+
text=f"Wrote {updated_rows} rows (including header) to {updated_range}.",
184+
)
185+
]
186+
except Exception as e:
187+
return [TextContent(type="text", text=_error_message(e))]
160188

161189

162190
@mcp.tool(
@@ -174,20 +202,23 @@ async def sheets_create(params: SheetsCreateInput) -> list[TextContent]:
174202
175203
Returns the spreadsheet ID and URL.
176204
"""
177-
token = await get_google_token()
178-
179-
async with GoogleSheetsClient(token) as client:
180-
metadata = await client.create_spreadsheet(params.title)
181-
spreadsheet_id = metadata["spreadsheetId"]
182-
url = metadata.get(
183-
"spreadsheetUrl",
184-
f"https://docs.google.com/spreadsheets/d/{spreadsheet_id}",
185-
)
205+
try:
206+
token = await get_google_token()
207+
208+
async with GoogleSheetsClient(token) as client:
209+
metadata = await client.create_spreadsheet(params.title)
210+
spreadsheet_id = metadata["spreadsheetId"]
211+
url = metadata.get(
212+
"spreadsheetUrl",
213+
f"https://docs.google.com/spreadsheets/d/{spreadsheet_id}",
214+
)
186215

187-
# Optionally populate with initial data
188-
if params.data:
189-
values = records_to_values(params.data)
190-
await client.write_range(spreadsheet_id, "Sheet1", values)
216+
# Optionally populate with initial data
217+
if params.data:
218+
values = records_to_values(params.data)
219+
await client.write_range(spreadsheet_id, "Sheet1", values)
220+
except Exception as e:
221+
return [TextContent(type="text", text=_error_message(e))]
191222

192223
result = {
193224
"spreadsheet_id": spreadsheet_id,
@@ -217,10 +248,13 @@ async def sheets_create(params: SheetsCreateInput) -> list[TextContent]:
217248
)
218249
async def sheets_info(params: SheetsInfoInput) -> list[TextContent]:
219250
"""Get metadata about a Google Sheet: title, sheet names, and dimensions."""
220-
token = await get_google_token()
251+
try:
252+
token = await get_google_token()
221253

222-
async with GoogleSheetsClient(token) as client:
223-
metadata = await client.get_spreadsheet_metadata(params.spreadsheet_id)
254+
async with GoogleSheetsClient(token) as client:
255+
metadata = await client.get_spreadsheet_metadata(params.spreadsheet_id)
256+
except Exception as e:
257+
return [TextContent(type="text", text=_error_message(e))]
224258

225259
title = metadata.get("properties", {}).get("title", "Unknown")
226260
sheets = []

0 commit comments

Comments
 (0)