-
Notifications
You must be signed in to change notification settings - Fork 43
[Feature] settle in parallel #693
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
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -5,7 +5,7 @@ import { calculateRefundAmount } from 'utils'; | |
| import { checkBalance } from 'services/BalanceCheckService'; | ||
| import { prisma } from 'server'; | ||
| import { makeProxyPassthroughRequest } from 'services/ProxyPassthroughService'; | ||
| import logger from 'logger'; | ||
| import logger, { logMetric } from 'logger'; | ||
| import { ProviderType } from 'providers/ProviderType'; | ||
| import { settle } from 'handlers/settle'; | ||
| import { finalize } from 'handlers/finalize'; | ||
|
|
@@ -24,55 +24,91 @@ export async function handleX402Request({ | |
| if (isPassthroughProxyRoute) { | ||
| return await makeProxyPassthroughRequest(req, res, provider, headers); | ||
| } | ||
| const settleResult = await settle(req, res, headers, maxCost); | ||
| if (!settleResult) { | ||
| return; | ||
| } | ||
|
|
||
| const { payload, paymentAmountDecimal } = settleResult; | ||
| const settlePromise = settle(req, res, headers, maxCost); | ||
|
|
||
| try { | ||
| const transactionResult = await modelRequestService.executeModelRequest( | ||
| req, | ||
| res, | ||
| headers, | ||
| provider, | ||
| isStream | ||
| ); | ||
| const transaction = transactionResult.transaction; | ||
| if (provider.getType() === ProviderType.OPENAI_VIDEOS) { | ||
| await prisma.videoGenerationX402.create({ | ||
| data: { | ||
| videoId: transaction.metadata.providerId, | ||
| wallet: payload.authorization.from, | ||
| cost: transaction.rawTransactionCost, | ||
| expiresAt: new Date(Date.now() + 1000 * 60 * 60 * 1), | ||
| }, | ||
| }); | ||
| } | ||
| const modelResultPromise = modelRequestService | ||
| .executeModelRequest(req, res, headers, provider, isStream) | ||
| .then((data) => ({ success: true as const, data })) | ||
| .catch((error) => ({ success: false as const, error: error as Error })); | ||
|
|
||
| const [settleResult, modelResult] = await Promise.all([ | ||
| settlePromise, | ||
| modelResultPromise, | ||
| ]); | ||
|
|
||
| // Case 1: Settle failed and model failed | ||
| if (!settleResult && !modelResult.success) { | ||
| return; | ||
| } | ||
|
|
||
| // Case 2: Settle failed but model succeeded | ||
| if (!settleResult && modelResult.success) { | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Case 2 attempts to send a response when one has already been sent by the settle function, causing an error. View Details📝 Patch Detailsdiff --git a/packages/app/server/src/handlers.ts b/packages/app/server/src/handlers.ts
index 131d35d9..d8af4084 100644
--- a/packages/app/server/src/handlers.ts
+++ b/packages/app/server/src/handlers.ts
@@ -54,11 +54,6 @@ export async function handleX402Request({
provider: provider.getType(),
url: req.url,
});
- modelRequestService.handleResolveResponse(
- res,
- isStream,
- data
- );
return;
}
AnalysisDouble response sent in handleX402Request when settle fails but model succeedsWhat fails: In How to reproduce: // In handleX402Request, Case 2 is triggered when:
// 1. settle() fails (e.g., getSmartAccount fails, validateXPaymentHeader fails, insufficient payment, etc.)
// - settle() calls buildX402Response() which sends res.status(402).json(resBody)
// - settle() returns undefined
// 2. AND modelRequestService.executeModelRequest() succeeds (Promise.all waits for both)
// Result: Both promises complete, Case 2 condition is true (!settleResult && modelResult.success)
// Code calls modelRequestService.handleResolveResponse(res, isStream, data)
// This attempts to call res.json() on an already-responded connectionWhat happens vs expected: Throws unhandled error "Cannot set headers after they are sent to the client" when attempting to send the second response. Expected behavior: Since Fix: Remove the |
||
| const { data } = modelResult; | ||
| logger.error('Settle failed but model request succeeded', { | ||
| provider: provider.getType(), | ||
| url: req.url, | ||
| metadata: data.transaction.metadata, | ||
| }); | ||
| logMetric('x402_request_settle_failed_model_request_succeeded', 1, { | ||
| provider: provider.getType(), | ||
| url: req.url, | ||
| }); | ||
| modelRequestService.handleResolveResponse( | ||
| res, | ||
| isStream, | ||
| transactionResult.data | ||
| data | ||
| ); | ||
| return; | ||
| } | ||
|
|
||
| logger.info( | ||
| `Creating X402 transaction for app. Metadata: ${JSON.stringify(transaction.metadata)}` | ||
| ); | ||
| const transactionCosts = | ||
| await x402AuthenticationService.createX402Transaction(transaction); | ||
|
|
||
| await finalize( | ||
| paymentAmountDecimal, | ||
| transactionCosts.rawTransactionCost, | ||
| transactionCosts.totalAppProfit, | ||
| transactionCosts.echoProfit, | ||
| payload | ||
| ); | ||
| } catch (error) { | ||
| // At this point, settleResult is guaranteed to exist | ||
| if (!settleResult) { | ||
| return; | ||
| } | ||
|
|
||
| const { payload, paymentAmountDecimal } = settleResult; | ||
|
|
||
| // Case 3: Settle succeeded but model failed | ||
| if (!modelResult.success) { | ||
| await refund(paymentAmountDecimal, payload); | ||
| return; | ||
| } | ||
|
|
||
| // Case 4: Both settle and model succeeded | ||
| const transactionResult = modelResult.data; | ||
| const transaction = transactionResult.transaction; | ||
|
|
||
| if (provider.getType() === ProviderType.OPENAI_VIDEOS) { | ||
| await prisma.videoGenerationX402.create({ | ||
| data: { | ||
| videoId: transaction.metadata.providerId, | ||
| wallet: payload.authorization.from, | ||
| cost: transaction.rawTransactionCost, | ||
| expiresAt: new Date(Date.now() + 1000 * 60 * 60 * 1), | ||
| }, | ||
| }); | ||
| } | ||
|
|
||
| modelRequestService.handleResolveResponse( | ||
| res, | ||
| isStream, | ||
| transactionResult.data | ||
| ); | ||
|
|
||
| logger.info( | ||
| `Creating X402 transaction for app. Metadata: ${JSON.stringify(transaction.metadata)}` | ||
| ); | ||
| const transactionCosts = | ||
| await x402AuthenticationService.createX402Transaction(transaction); | ||
|
|
||
| await finalize( | ||
| paymentAmountDecimal, | ||
| transactionCosts.rawTransactionCost, | ||
| transactionCosts.totalAppProfit, | ||
| transactionCosts.echoProfit, | ||
| payload | ||
| ); | ||
|
Comment on lines
+105
to
+111
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. looks like previously we would catch and do the refund if this threw. is this intentional? aren't we missing the outer try/catch
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. We could try/catch here, again if this fails it will throw all the way up to the root and return an error. Do you think we need to re-throw in this?
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. the case im thinking about is
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. if settle succeeds and model request fails that would catch in a different branch. In that case we gave out tokens for free, we wouldn't refund or finalize. if settle succeds and model fails, we just refund. If both succeed, we can just fire the finalize and forget. If finalize fails, the error will bubble up to the next() call and return with the correct error message |
||
| } | ||
|
|
||
| export async function handleApiKeyRequest({ | ||
|
|
||
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.
just return null here works ? upstream stuff handles this correctly?
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.
If both fail, the model's error response should throw all the way up to the root and return with that