-
-
Notifications
You must be signed in to change notification settings - Fork 114
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
Add timeouts to all wallet API calls #1722
Changes from all commits
ee8191c
725b052
74f3610
e624e1c
8d3b136
8093be3
cf95c54
456df5a
eff81eb
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 |
---|---|---|
|
@@ -2,30 +2,44 @@ import fetch from 'cross-fetch' | |
import crypto from 'crypto' | ||
import { getAgent } from '@/lib/proxy' | ||
import { assertContentTypeJson, assertResponseOk } from './url' | ||
import { FetchTimeoutError } from './fetch' | ||
import { WALLET_CREATE_INVOICE_TIMEOUT_MS } from './constants' | ||
|
||
export const createInvoice = async ({ msats, description, expiry }, { socket, rune, cert }) => { | ||
export const createInvoice = async ({ msats, description, expiry }, { socket, rune, cert }, { signal }) => { | ||
const agent = getAgent({ hostname: socket, cert }) | ||
|
||
const url = `${agent.protocol}//${socket}/v1/invoice` | ||
const res = await fetch(url, { | ||
method: 'POST', | ||
headers: { | ||
'Content-Type': 'application/json', | ||
Rune: rune, | ||
// can be any node id, only required for CLN v23.08 and below | ||
// see https://docs.corelightning.org/docs/rest#server | ||
nodeId: '02cb2e2d5a6c5b17fa67b1a883e2973c82e328fb9bd08b2b156a9e23820c87a490' | ||
}, | ||
agent, | ||
body: JSON.stringify({ | ||
// CLN requires a unique label for every invoice | ||
// see https://docs.corelightning.org/reference/lightning-invoice | ||
label: crypto.randomBytes(16).toString('hex'), | ||
description, | ||
amount_msat: msats, | ||
expiry | ||
|
||
let res | ||
try { | ||
res = await fetch(url, { | ||
method: 'POST', | ||
headers: { | ||
'Content-Type': 'application/json', | ||
Rune: rune, | ||
// can be any node id, only required for CLN v23.08 and below | ||
// see https://docs.corelightning.org/docs/rest#server | ||
nodeId: '02cb2e2d5a6c5b17fa67b1a883e2973c82e328fb9bd08b2b156a9e23820c87a490' | ||
}, | ||
agent, | ||
body: JSON.stringify({ | ||
// CLN requires a unique label for every invoice | ||
// see https://docs.corelightning.org/reference/lightning-invoice | ||
label: crypto.randomBytes(16).toString('hex'), | ||
description, | ||
amount_msat: msats, | ||
expiry | ||
}), | ||
signal | ||
}) | ||
}) | ||
} catch (err) { | ||
if (err.name === 'AbortError') { | ||
// XXX node-fetch doesn't throw our custom TimeoutError but throws a generic error so we have to handle that manually. | ||
// see https://github.com/node-fetch/node-fetch/issues/1462 | ||
throw new FetchTimeoutError('POST', url, WALLET_CREATE_INVOICE_TIMEOUT_MS) | ||
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. this assumes the signal was a timeout signal that used |
||
} | ||
throw err | ||
} | ||
|
||
assertResponseOk(res) | ||
assertContentTypeJson(res) | ||
|
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.
Mhhh, if the wallet can use signals, then it's not clear if
withTimeout
or the signal will throw first 🤔Preferably, we want the signal to throw first so we get
FetchTimeoutError
as the error which includes additional information.I tested this with LNbits and setting the timeout to 10 and the signal always won but not sure if we can rely on that. I'll add 100ms to
withTimeout
to be sure but without affecting the timeout message itself.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.
done in eff81eb