diff --git a/client/client_content.go b/client/client_content.go index 77876b09..fd768117 100644 --- a/client/client_content.go +++ b/client/client_content.go @@ -779,6 +779,8 @@ func (c *Client) handleFTGetChunk(ru *RemoteUser, gc rpc.RMFTGetChunk) error { var f clientdb.SharedFile var md rpc.FileMetadata var inv string + var amountMAtoms uint64 + var genInvErr error chunkIdx := gc.Index err := c.dbUpdate(func(tx clientdb.ReadWriteTx) error { var err error @@ -793,7 +795,7 @@ func (c *Client) handleFTGetChunk(ru *RemoteUser, gc rpc.RMFTGetChunk) error { } // Generate invoice for the given amount. - amountMAtoms := clientintf.FileChunkMAtoms(chunkIdx, &md) + amountMAtoms = clientintf.FileChunkMAtoms(chunkIdx, &md) if amountMAtoms < 1000 { // File is free to download. return nil @@ -819,8 +821,32 @@ func (c *Client) handleFTGetChunk(ru *RemoteUser, gc rpc.RMFTGetChunk) error { inv, err = c.genInvoiceForFTUpload(tx, ru, f, cid, chunkIdx, amountMAtoms) + if err != nil { + genInvErr = err + } return err }) + if genInvErr != nil { + // Generating the invoice for this chunk failed (e.g. dcrlnd has + // no inbound capacity). Notify the local user and reply to the + // getter with an error so its download does not stall silently. + errStr := genInvErr.Error() + c.ntfns.notifyInvoiceGenFailed(ru, float64(amountMAtoms)/1e11, genInvErr) + go func() { + rm := rpc.RMFTPayForChunk{ + Tag: gc.Tag, + FileID: gc.FileID, + Index: chunkIdx, + Hash: gc.Hash, + Error: &errStr, + } + if e := ru.sendRM(rm, "ftpayforchunk"); e != nil && + !errors.Is(e, clientintf.ErrSubsysExiting) { + ru.log.Errorf("Unable to send chunk-invoice failure: %v", e) + } + }() + return nil + } if err != nil { return err } @@ -838,6 +864,32 @@ func (c *Client) handleFTGetChunk(ru *RemoteUser, gc rpc.RMFTGetChunk) error { // handleFTPayForChunk this is called when we receive an invoice to pay for a // chunk of data. func (c *Client) handleFTPayForChunk(ru *RemoteUser, pfc rpc.RMFTPayForChunk) error { + // If the remote could not provide the chunk (e.g. it failed to generate + // an invoice), surface the failure instead of trying to decode an empty + // invoice, which would otherwise leave the download stalled. + if pfc.Error != nil { + derr := fmt.Errorf("remote could not provide chunk %d of file %s: %s", + pfc.Index, pfc.FileID, *pfc.Error) + ru.log.Warnf("%v", derr) + var fid clientdb.FileID + if err := fid.FromString(pfc.FileID); err != nil { + return err + } + var fd clientdb.FileDownload + err := c.dbView(func(tx clientdb.ReadTx) error { + var err error + fd, err = c.db.ReadFileDownload(tx, ru.ID(), fid) + return err + }) + if err != nil { + return err + } + if fd.Metadata != nil { + c.ntfns.notifyFileDownloadFailed(ru, *fd.Metadata, derr) + } + return nil + } + // Verify this is a valid invoice. inv, err := c.pc.DecodeInvoice(c.ctx, pfc.Invoice) if err != nil { diff --git a/client/notifications.go b/client/notifications.go index 43e5c702..456127cc 100644 --- a/client/notifications.go +++ b/client/notifications.go @@ -326,6 +326,14 @@ type OnFileDownloadProgress func(user *RemoteUser, fm rpc.FileMetadata, nbMissin func (OnFileDownloadProgress) typ() string { return onFileDownloadProgress } +const onFileDownloadFailed = "onFileDownloadFailed" + +// FileDownloadFailed is called when a file download cannot proceed, for +// example because the remote client could not generate an invoice for a chunk. +type OnFileDownloadFailed func(user *RemoteUser, fm rpc.FileMetadata, err error) + +func (OnFileDownloadFailed) typ() string { return onFileDownloadFailed } + const onRMReceived = "onRMReceived" // OnRMReceived is a notification sent whenever a remote user receives an RM. @@ -1127,6 +1135,11 @@ func (nmgr *NotificationManager) notifyFileDownloadProgress(user *RemoteUser, fm visit(func(h OnFileDownloadProgress) { h(user, fm, nbMissingChunks) }) } +func (nmgr *NotificationManager) notifyFileDownloadFailed(user *RemoteUser, fm rpc.FileMetadata, err error) { + nmgr.handlers[onFileDownloadFailed].(*handlersFor[OnFileDownloadFailed]). + visit(func(h OnFileDownloadFailed) { h(user, fm, err) }) +} + func (nmgr *NotificationManager) notifyRMReceived(ru *RemoteUser, rmh *rpc.RMHeader, p interface{}, ts time.Time) { nmgr.handlers[onRMReceived].(*handlersFor[OnRMReceived]). visit(func(h OnRMReceived) { h(ru, rmh, p, ts) }) @@ -1338,6 +1351,7 @@ func NewNotificationManager() *NotificationManager { onContentListReceived: &handlersFor[OnContentListReceived]{}, onFileDownloadCompleted: &handlersFor[OnFileDownloadCompleted]{}, onFileDownloadProgress: &handlersFor[OnFileDownloadProgress]{}, + onFileDownloadFailed: &handlersFor[OnFileDownloadFailed]{}, onServerUnwelcomeError: &handlersFor[OnServerUnwelcomeError]{}, onRequestingMediateIDType: &handlersFor[OnRequestingMediateID]{}, diff --git a/internal/e2etests/content_test.go b/internal/e2etests/content_test.go index 7ff2b8c0..773069b3 100644 --- a/internal/e2etests/content_test.go +++ b/internal/e2etests/content_test.go @@ -136,6 +136,48 @@ func TestFtDownloadFile(t *testing.T) { assert.EqualFiles(t, fGlobal, completedPath1) } +// TestFtPaidDownloadInvoiceGenFails verifies that when a host cannot generate +// the invoice for a paid file chunk, it signals the failure to both sides +// (OnInvoiceGenFailedNtfn locally and an error reply that the getter surfaces +// as OnFileDownloadFailed) rather than leaving the getter's download stalled. +func TestFtPaidDownloadInvoiceGenFails(t *testing.T) { + t.Parallel() + + tcfg := testScaffoldCfg{} + ts := newTestScaffold(t, tcfg) + alice := ts.newClient("alice") + bob := ts.newClient("bob") + ts.kxUsers(alice, bob) + + // The host (alice) cannot generate an invoice for the chunk. + alice.mpc.HookGetInvoice(func(amt int64, cb func(int64)) (string, error) { + return "", fmt.Errorf("not enough inbound capacity") + }) + + // Host should be notified the invoice could not be generated. + aliceInvFailChan := make(chan error, 4) + alice.handle(client.OnInvoiceGenFailedNtfn(func(_ *client.RemoteUser, _ float64, err error) { + aliceInvFailChan <- err + })) + // Getter should be notified the download failed (instead of stalling). + bobDownFailChan := make(chan error, 4) + bob.handle(client.OnFileDownloadFailed(func(_ *client.RemoteUser, _ rpc.FileMetadata, err error) { + bobDownFailChan <- err + })) + + // Alice shares a paid file with Bob. + f := testutils.RandomFile(t, defaultChunkSize) + bobUID := bob.PublicID() + sf, _, err := alice.ShareFile(f, &bobUID, 1, "paid file") + assert.NilErr(t, err) + + // Bob requests it; the host's invoice generation fails. Instead of + // stalling, both sides receive a signal. + assert.NilErr(t, bob.GetUserContent(alice.PublicID(), sf.FID)) + assert.ChanWritten(t, aliceInvFailChan) + assert.ChanWritten(t, bobDownFailChan) +} + // TestFtSendFile tests that the send file feature works. func TestFtSendFile(t *testing.T) { t.Parallel()