-
Notifications
You must be signed in to change notification settings - Fork 22
Add Tavily Tool-calls #541
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.
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
Conversation
The latest updates on your projects. Learn more about Vercel for GitHub.
1 Skipped Deployment
|
🚅 Deployed to the echo-pr-541 environment in echo
|
metadata: { | ||
providerId: output.request_id, | ||
provider: 'tavily', | ||
model: input.search_depth ?? 'basic', |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
is this right?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Probably not, but it will get pretty annoying to always add n+1 resources if all usage is going to be A. recorded in echo tx and B. need it's own fields.
We should abandon supporting echo balance or just be okay with overloading
input: TavilySearchInput, | ||
output: TavilySearchOutput, | ||
cost: Decimal | ||
): Transaction => { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this whole tx object is throwing me off. why do we need this for x402? (or are we allowing echo credits)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We're allowing creds
buildX402Response(req, res, maxCost); | ||
return undefined; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
i dont get this change. we are no longer returning the 402 response?
packages/app/server/src/handlers.ts
Outdated
await transfer( | ||
authPayload.from as `0x${string}`, | ||
refundAmountUsdcBigInt | ||
).catch(transferError => { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
is this just a promise returning a promise? why are we .catch()
buildX402Response(req, res, maxCost); | ||
return; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
i may have introduced this pattern to fix the 502 thing but we should prob bubble the 402 response up instead of writing to res deep in this call stack then return nothing
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
agreed but no reason to block this PR on this, it's an issue in the codebase otherwise
} else if (isX402Request(headers)) { | ||
const settleResult = await settle(req, res, headers, maxCost); | ||
if (!settleResult) { | ||
return buildX402Response(req, res, maxCost); | ||
} | ||
const { payload, paymentAmountDecimal } = settleResult; | ||
|
||
const output = await tavilySearch(inputBody); | ||
|
||
const transaction = createTavilyTransaction(inputBody, output, maxCost); | ||
|
||
await finalize(paymentAmountDecimal, transaction, payload); | ||
|
||
return res.status(200).json(output); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Missing error handling for X402 payments - if the Tavily API call fails after payment settlement, users won't receive refunds.
View Details
📝 Patch Details
diff --git a/packages/app/server/src/resources/tavily/route.ts b/packages/app/server/src/resources/tavily/route.ts
index b8f9c685..cb322777 100644
--- a/packages/app/server/src/resources/tavily/route.ts
+++ b/packages/app/server/src/resources/tavily/route.ts
@@ -1,10 +1,10 @@
-import { buildX402Response, isApiRequest, isX402Request } from 'utils';
+import { buildX402Response, isApiRequest, isX402Request, decimalToUsdcBigInt } from 'utils';
import { TavilySearchInputSchema } from './types';
import { calculateTavilySearchCost, tavilySearch } from './tavily';
import { authenticateRequest } from 'auth';
import { prisma } from 'server';
-import { settle } from 'handlers';
-import { finalize } from 'handlers';
+import { settle, finalize } from 'handlers';
+import { transfer } from 'transferWithAuth';
import { createTavilyTransaction } from './tavily';
import logger from 'logger';
import { Request, Response } from 'express';
@@ -38,13 +38,21 @@ export async function tavilySearchRoute(req: Request, res: Response) {
}
const { payload, paymentAmountDecimal } = settleResult;
- const output = await tavilySearch(inputBody);
+ try {
+ const output = await tavilySearch(inputBody);
- const transaction = createTavilyTransaction(inputBody, output, maxCost);
+ const transaction = createTavilyTransaction(inputBody, output, maxCost);
- await finalize(paymentAmountDecimal, transaction, payload);
+ await finalize(paymentAmountDecimal, transaction, payload);
- return res.status(200).json(output);
+ return res.status(200).json(output);
+ } catch (error) {
+ // Full refund on error, like the main handleX402Request handler does
+ const refundAmountUsdcBigInt = decimalToUsdcBigInt(paymentAmountDecimal);
+ const authPayload = payload.authorization;
+ await transfer(authPayload.from as `0x${string}`, refundAmountUsdcBigInt);
+ throw error;
+ }
} else {
return buildX402Response(req, res, maxCost);
}
Analysis
Missing error handling in tavily X402 route causes payment loss on API failures
What fails: tavilySearchRoute()
in X402 payment path lacks error handling after settle()
- if tavilySearch()
throws an error, user loses payment without receiving service
How to reproduce:
# Send X402 payment request to tavily endpoint with invalid/expired API key
curl -X POST /tavily/search \
-H "Content-Type: application/json" \
-H "X-Payment: <valid_payment_payload>" \
-d '{"query": "test", "search_depth": "basic"}'
Result: Payment gets settled successfully, then tavilySearch()
throws "Tavily API request failed: 401 Unauthorized" but user receives no refund - payment is lost
Expected: Should perform full refund like handleX402Request()
does at handlers.ts:156-160 when errors occur after settlement
Root cause: Tavily route calls settle()
, then tavilySearch()
without try-catch protection, unlike main X402 handler which wraps post-settlement operations in try-catch with transfer()
refund on errors
Shows how easy it is to add arbitrary upstream tools to the router.
There might be a better interface... should it proxy? etc.
Working well for 402 for now