Skip to content

Commit

Permalink
fix: Improve error handling when processig callback calls between JS …
Browse files Browse the repository at this point in the history 
…and RS.
  • Loading branch information
@fmarek-kindred
fmarek-kindred committed Sep 19, 2023
1 parent fea08e3 commit 734d462
Show file tree
Hide file tree
Showing 6 changed files with 50 additions and 87 deletions.
26 changes: 19 additions & 7 deletions cohort_banking_initiator_js/src/banking-app.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ import { logger } from "./logger"

import { CapturedItemState, CapturedState, TransferRequest, TransferRequestMessage } from "./model"
import { SDK_CONFIG as sdkConfig } from "./cfg/config-cohort-sdk"
import { Initiator, JsCertificationRequestPayload, OutOfOrderRequest } from "cohort_sdk_client"
import { Initiator, JsCertificationRequestPayload, JsOutOfOrderInstallOutcome, OutOfOrderRequest, TalosSdkError } from "cohort_sdk_client"

export class BankingApp {
private startedAtMs: number = 0
Expand Down Expand Up @@ -60,7 +60,17 @@ export class BankingApp {
const subSpans = await this.handleTransaction(event.data.request)
spans.processDetails = subSpans
} catch (e) {
logger.error("Unable to process tx: %s. Error: %s", JSON.stringify(event.data), e)
if (e instanceof TalosSdkError) {
const sdkError = e as TalosSdkError
logger.error("Unable to process tx: %s. TalosSdkError", JSON.stringify(event.data))
logger.error("TalosSdkError.message: %s", sdkError.message)
logger.error("TalosSdkError.kind: %s", sdkError.kind)
logger.error("TalosSdkError.name: %s", sdkError.name)
logger.error("TalosSdkError.cause: %s", sdkError.cause)
logger.error("TalosSdkError.stack: %s", sdkError.stack)
} else {
logger.error("Unable to process tx: %s. Error: %s", JSON.stringify(event.data), e)
}
} finally {
this.handledCount++
spans.process = Date.now() - span_s
Expand Down Expand Up @@ -148,14 +158,15 @@ export class BankingApp {
// take snapshot from any row
return new CapturedState(Number(result.rows[0].snapshot_version), items)
} catch (e) {
// This print here is important, without it the original reason is lost when using NAPI 2.10.
logger.error("BankingApp.loadState(): %s", e)
throw new Error(`Unable to load state for tx: ${ JSON.stringify(tx) }`, { cause: e })
throw new Error(`Unable to load state for tx: ${ JSON.stringify(tx) }. Reason: ${e.message}`, { cause: e })
} finally {
cnn?.release()
}
}

private async installOutOfOrder(tx: TransferRequest, request: OutOfOrderRequest): Promise<number> {
private async installOutOfOrder(tx: TransferRequest, request: OutOfOrderRequest): Promise<JsOutOfOrderInstallOutcome> {
let cnn: PoolClient
try {
// Params order:
Expand Down Expand Up @@ -189,20 +200,21 @@ export class BankingApp {

if (result.rowCount === 0) {
// installed already
return 1
return JsOutOfOrderInstallOutcome.InstalledAlready
}

// Quickly grab the snapshot to check whether safepoint condition is satisfied. Any row can be used for that.
const snapshot = Number(result.rows[0].snapshot)
if (snapshot < request.safepoint) {
// safepoint condition
return 2
return JsOutOfOrderInstallOutcome.SafepointCondition
}

// installed
return 0
return JsOutOfOrderInstallOutcome.Installed

} catch (e) {
// This print here is important, without it the original reason is lost when using NAPI 2.10.
logger.error("BankingApp.installOutOfOrder(): %s", e)
throw new Error(`Unable to complete out of order installation of tx: ${ JSON.stringify(tx) }`, { cause: e })
} finally {
Expand Down
39 changes: 0 additions & 39 deletions cohort_banking_initiator_js/src/main.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -47,45 +47,6 @@ class LaunchParams {
new Promise(async (resolve) => {
const params = LaunchParams.parse(process.argv)

// try {
// new SomeRustServiceClass().testCatchWrapAndThrow()
// } catch (e) {
// console.log(e.message)
// console.log(e)
// console.log(e.cause)
// return
// }

// try {
// new SomeRustServiceClass().example1DirectThrow(100)
// } catch (e) {
// logger.info("- - - - - - - - App caught error - - - - - - - - - - - -")
// if (e instanceof TalosSdkError) {
// if (e.code === 100) {
// logger.error("Caught TalosSdkError with code 100: %s\n%s\n%s", e.message, e, e.cause)
// } else {
// logger.error("Caught TalosSdkError with unexpected code: %s. Error: %s", e.code, e)
// }
// } else {
// logger.error("Caught some generic Error:\ndetails:\n\t%s", e)
// }
// logger.info("- - - - - - - - - - - - - - - - - - - - - - - - - -")
// throw e
// }

// try {
// new SomeRustServiceClass().example2SimulateRustError()
// } catch (e) {
// logger.info("- - - - - - - - App caught error - - - - - - - - - - - -")
// if (e instanceof TalosSdkError) {
// logger.error("Caught TalosSdkError:\n%s\n%s", e.message, e, e.cause)
// } else {
// logger.error("Caught some generic Error:\ndetails:\n\t%s", e)
// }
// logger.info("- - - - - - - - - - - - - - - - - - - - - - - - - -")
// throw e
// }

const database = new Pool(DB_CONFIG)
database.on("error", (e, _) => { logger.error("DBPool.error: Error: %s", e) })
database.on("release", (e, _) => { if (e) { logger.error("DBPool.release: Error: %s", e) } })
Expand Down
2 changes: 2 additions & 0 deletions cohort_sdk_client/src/index.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ import { Initiator } from "./initiator"
import {
JsCertificationRequestPayload,
JsInitiatorConfig,
JsOutOfOrderInstallOutcome,
OutOfOrderRequest,
SdkErrorKind,
} from "cohort_sdk_js"
Expand All @@ -16,6 +17,7 @@ export {
Initiator,
JsInitiatorConfig,
JsCertificationRequestPayload,
JsOutOfOrderInstallOutcome,
OutOfOrderRequest,
SdkErrorKind,
TalosSdkError,
Expand Down
41 changes: 29 additions & 12 deletions packages/cohort_sdk_js/src/initiator/mod.rs
View file
Original file line number Diff line number Diff line change
@@ -1,4 +1,3 @@
use crate::map_error_to_napi_error;
use crate::models::{JsBackoffConfig, JsKafkaConfig};
use crate::sdk_errors::SdkErrorContainer;
use async_trait::async_trait;
Expand All @@ -8,7 +7,9 @@ use cohort_sdk::model::callback::{
OutOfOrderInstaller,
};
use cohort_sdk::model::{ClientError, Config};
use napi::bindgen_prelude::FromNapiValue;
use napi::bindgen_prelude::Promise;
use napi::bindgen_prelude::ToNapiValue;
use napi::threadsafe_function::ThreadsafeFunction;
use napi_derive::napi;
use serde_json::Value;
Expand Down Expand Up @@ -108,6 +109,23 @@ pub struct JsCertificationCandidateCallbackResponse {
pub new_request: Option<JsCertificationRequestPayload>,
}

#[napi(string_enum)]
pub enum JsOutOfOrderInstallOutcome {
Installed,
InstalledAlready,
SafepointCondition,
}

impl From<JsOutOfOrderInstallOutcome> for OutOfOrderInstallOutcome {
fn from(value: JsOutOfOrderInstallOutcome) -> Self {
match value {
JsOutOfOrderInstallOutcome::Installed => OutOfOrderInstallOutcome::Installed,
JsOutOfOrderInstallOutcome::SafepointCondition => OutOfOrderInstallOutcome::SafepointCondition,
JsOutOfOrderInstallOutcome::InstalledAlready => OutOfOrderInstallOutcome::InstalledAlready,
}
}
}

#[napi]
pub struct InternalInitiator {
cohort: Cohort,
Expand Down Expand Up @@ -148,17 +166,13 @@ impl OutOfOrderInstaller for OutOfOrderInstallerImpl {
new_version: request.version.try_into().unwrap(),
};

let result = self.ooo_callback.call_async::<Promise<i64>>(Ok(oorequest)).await;
let result = self.ooo_callback.call_async::<Promise<JsOutOfOrderInstallOutcome>>(Ok(oorequest)).await;

match result {
Ok(promise) => promise
.await
.map(|code| match code {
1 => OutOfOrderInstallOutcome::InstalledAlready,
2 => OutOfOrderInstallOutcome::SafepointCondition,
_ => OutOfOrderInstallOutcome::Installed,
})
.map_err(|e| e.to_string()),
.map(|outcome| outcome.into())
.map_err(|e| format!("Unable to install out of orer item. Native reason provided by JS: \"{}\"", e.reason)),
Err(e) => Err(e.to_string()),
}
}
Expand All @@ -173,8 +187,8 @@ impl NewRequestProvider {
let result = self
.make_new_request_callback
.call_async::<Promise<JsCertificationCandidateCallbackResponse>>(Ok(()))
.await
.map_err(map_error_to_napi_error);
.await;

match result {
Ok(promise) => promise
.await
Expand All @@ -185,12 +199,15 @@ impl NewRequestProvider {
CertificationCandidateCallbackResponse::Proceed(
js_data
.new_request
.expect("Invalid response from 'get_state_callback'. Provide cancellation reason or new request. Currently both are empty.")
.expect(
"Invalid response from 'make_new_request_callback'. Provide cancellation reason or new request. Currently both are empty.",
)
.into(),
)
}
})
.map_err(|e| e.to_string()),
// Here reason is empty with NAPI 2.10.3
.map_err(|e| format!("Unable to create new certification request. Native reason reported from JS: \"{}\"", e.reason)),

Err(e) => Err(e.to_string()),
}
Expand Down
4 changes: 0 additions & 4 deletions packages/cohort_sdk_js/src/lib.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -4,11 +4,7 @@ pub mod initiator;
pub mod installer;
pub mod models;
pub mod sdk_errors;
pub mod typed_errors_test;

fn map_error_to_napi_error<T: Display>(e: T) -> napi::Error {
napi::Error::from_reason(e.to_string())
}

// #[macro_use]
// extern crate napi_derive;
25 changes: 0 additions & 25 deletions packages/cohort_sdk_js/src/typed_errors_test.rs
View file

This file was deleted.

0 comments on commit 734d462

Please sign in to comment.