fix(peer): handle UploadDenied so refused downloads fail fast
Peer message code 50 had no handler, so the dispatcher dropped it and a download refused by the peer stayed Queued until the caller's timeout. The new handler reads the filename and reason and routes the denial into the same failure path as UploadFailed. A reason of "Queued" is status, not a failure: some clients send it for a download that is still held in their queue, and failing it would kill healthy downloads after a reconnect, so that one is only logged. Fixes #29
This commit is contained in:
@@ -5,7 +5,7 @@ use crate::message::peer::{
|
|||||||
FileSearchResponse, GetShareFileList, PeerInit, PlaceInQueueRequest,
|
FileSearchResponse, GetShareFileList, PeerInit, PlaceInQueueRequest,
|
||||||
PlaceInQueueResponse, QueueUploadHandler, SharedDirectory,
|
PlaceInQueueResponse, QueueUploadHandler, SharedDirectory,
|
||||||
SharedFileListResponseHandler, TransferRequest, TransferResponse,
|
SharedFileListResponseHandler, TransferRequest, TransferResponse,
|
||||||
UploadFailedHandler,
|
UploadDeniedHandler, UploadFailedHandler,
|
||||||
};
|
};
|
||||||
use crate::message::server::MessageFactory;
|
use crate::message::server::MessageFactory;
|
||||||
use crate::message::{Handlers, Message, MessageReader, MessageType};
|
use crate::message::{Handlers, Message, MessageReader, MessageType};
|
||||||
@@ -178,6 +178,7 @@ impl PeerActor {
|
|||||||
handlers.register_handler(TransferRequest);
|
handlers.register_handler(TransferRequest);
|
||||||
handlers.register_handler(TransferResponse);
|
handlers.register_handler(TransferResponse);
|
||||||
handlers.register_handler(GetShareFileList);
|
handlers.register_handler(GetShareFileList);
|
||||||
|
handlers.register_handler(UploadDeniedHandler);
|
||||||
handlers.register_handler(UploadFailedHandler);
|
handlers.register_handler(UploadFailedHandler);
|
||||||
handlers.register_handler(PlaceInQueueRequest);
|
handlers.register_handler(PlaceInQueueRequest);
|
||||||
handlers.register_handler(PlaceInQueueResponse);
|
handlers.register_handler(PlaceInQueueResponse);
|
||||||
|
|||||||
@@ -7,6 +7,7 @@ mod queue_upload;
|
|||||||
mod shared_file_list;
|
mod shared_file_list;
|
||||||
mod transfer_request;
|
mod transfer_request;
|
||||||
mod transfer_response;
|
mod transfer_response;
|
||||||
|
mod upload_denied;
|
||||||
mod upload_failed;
|
mod upload_failed;
|
||||||
|
|
||||||
// Re-export handlers
|
// Re-export handlers
|
||||||
@@ -24,4 +25,5 @@ pub use shared_file_list::{
|
|||||||
};
|
};
|
||||||
pub use transfer_request::TransferRequest;
|
pub use transfer_request::TransferRequest;
|
||||||
pub use transfer_response::TransferResponse;
|
pub use transfer_response::TransferResponse;
|
||||||
|
pub use upload_denied::UploadDeniedHandler;
|
||||||
pub use upload_failed::UploadFailedHandler;
|
pub use upload_failed::UploadFailedHandler;
|
||||||
|
|||||||
@@ -0,0 +1,56 @@
|
|||||||
|
use crate::info;
|
||||||
|
use crate::{
|
||||||
|
message::{Message, MessageHandler},
|
||||||
|
peer::PeerMessage,
|
||||||
|
};
|
||||||
|
use std::sync::mpsc::Sender;
|
||||||
|
|
||||||
|
pub struct UploadDeniedHandler;
|
||||||
|
impl MessageHandler<PeerMessage> for UploadDeniedHandler {
|
||||||
|
fn get_code(&self) -> u8 {
|
||||||
|
50
|
||||||
|
}
|
||||||
|
fn handle(&self, message: &mut Message, sender: Sender<PeerMessage>) {
|
||||||
|
let filename = message.read_string();
|
||||||
|
let reason = message.read_string();
|
||||||
|
info!("Upload denied for {}: {}", filename, reason);
|
||||||
|
if reason == "Queued" {
|
||||||
|
return;
|
||||||
|
}
|
||||||
|
let _ = sender.send(PeerMessage::UploadFailed(String::new(), filename));
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
#[cfg(test)]
|
||||||
|
mod tests {
|
||||||
|
use super::*;
|
||||||
|
use crate::message::framed;
|
||||||
|
|
||||||
|
#[test]
|
||||||
|
fn a_queued_reason_is_not_a_failure() {
|
||||||
|
let (tx, rx) = std::sync::mpsc::channel();
|
||||||
|
let mut message = framed(|m| {
|
||||||
|
m.write_string("@@share\\gone.mp3").write_string("Queued");
|
||||||
|
});
|
||||||
|
|
||||||
|
UploadDeniedHandler.handle(&mut message, tx);
|
||||||
|
assert!(rx.try_recv().is_err());
|
||||||
|
}
|
||||||
|
|
||||||
|
#[test]
|
||||||
|
fn the_denied_filename_is_forwarded() {
|
||||||
|
let (tx, rx) = std::sync::mpsc::channel();
|
||||||
|
let mut message = framed(|m| {
|
||||||
|
m.write_string("@@share\\gone.mp3")
|
||||||
|
.write_string("File not shared.");
|
||||||
|
});
|
||||||
|
|
||||||
|
UploadDeniedHandler.handle(&mut message, tx);
|
||||||
|
match rx.try_recv() {
|
||||||
|
Ok(PeerMessage::UploadFailed(_, filename)) => {
|
||||||
|
assert_eq!(filename, "@@share\\gone.mp3");
|
||||||
|
}
|
||||||
|
other => panic!("unexpected: {other:?}"),
|
||||||
|
}
|
||||||
|
}
|
||||||
|
}
|
||||||
@@ -859,6 +859,18 @@ fn an_upload_failed_reply_fails_the_download_quickly() {
|
|||||||
});
|
});
|
||||||
}
|
}
|
||||||
|
|
||||||
|
#[test]
|
||||||
|
fn an_upload_denied_reply_fails_the_download_quickly() {
|
||||||
|
refusal_fails_the_download_quickly("e2e_denying_peer", |filename| {
|
||||||
|
let mut refusal = Message::new();
|
||||||
|
refusal
|
||||||
|
.write_int32(50)
|
||||||
|
.write_string(filename)
|
||||||
|
.write_string("File not shared.");
|
||||||
|
refusal
|
||||||
|
});
|
||||||
|
}
|
||||||
|
|
||||||
#[test]
|
#[test]
|
||||||
fn an_interrupted_download_resumes_from_its_partial_file() {
|
fn an_interrupted_download_resumes_from_its_partial_file() {
|
||||||
let server = server_or_skip!();
|
let server = server_or_skip!();
|
||||||
|
|||||||
Reference in New Issue
Block a user