Skip to content
This repository has been archived by the owner on Aug 16, 2023. It is now read-only.

Add janus requests timeout #159

Merged
merged 4 commits into from
Sep 30, 2020
Merged

Add janus requests timeout #159

merged 4 commits into from
Sep 30, 2020

Conversation

feymartynov
Copy link
Contributor

No description provided.

src/app/janus.rs Outdated
@@ -34,6 +37,135 @@ use crate::util::{from_base64, generate_correlation_data, to_base64};

const STREAM_UPLOAD_METHOD: &str = "stream.upload";

lazy_static! {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Let's move these parameters to the app configuration. [backend] section for example.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[backend] section would also be a nice place for backend_id, meaning

[backend]
id = "janus-gateway.svc.example.org"

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

src/backend would probably be a better place for this entire file.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done. Also refactored it into smaller modules.

@@ -67,6 +76,7 @@ pub(crate) trait Context: Sync {
fn config(&self) -> &Config;
fn db(&self) -> &Db;
fn agent_id(&self) -> &AgentId;
fn janus_client(&self) -> &Arc<JanusClient>;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

why return pointer to arc?

payload.handle_id.janus_handle_id(),
payload.handle_id.rtc_id(),
payload.jsep.clone(),
payload.handle_id.backend_id(),
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

is it ok to pass 4 args coming from payload.handle_id instead of just payload.handle_id itself?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No, it's not OK. There's a lot of room for refactoring. I've just moved code as is for now.

authz_time,
)
.map(|req| Box::new(req) as Box<dyn IntoPublishableMessage + Send>)
.map_err(|err| format!("error creating a backend request: {}", err))
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

{:?} here

authz_time,
)
.map(|req| Box::new(req) as Box<dyn IntoPublishableMessage + Send>)
.map_err(|err| format!("error creating a backend request: {}", err))
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

{:?} here

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same about error. There's another task for improving them.

backend.id(),
start_timestamp,
)
.map_err(|err| format!("error creating a backend request: {}", err))
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

{:?}

.build();

sentry::send(svc_error).unwrap_or_else(|err| {
warn!("Error sending error to Sentry: {}", err)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

{:?}

reqp.correlation_data().to_owned(),
request_info,
))
.unwrap_or_else(|err| error!("Failed to register janus client transaction: {}", err));
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

{:?}


self.transaction_watchdog_tx
.send(TransactionWatchdogMessage::Remove(corr_data))
.unwrap_or_else(|err| error!("Failed to remove janus client transaction: {}", err));
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

{:?}

let period = StdDuration::from_secs(config.transaction_watchdog_check_period);
let (tx, rx) = crossbeam_channel::unbounded();

thread::spawn(move || {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@feymartynov feymartynov merged commit 183eb7a into master Sep 30, 2020
@feymartynov feymartynov deleted the feature/ULMS-817 branch September 30, 2020 14:15
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants