Skip to content

Commit

Permalink
UX improvements: FFI logging, error message fix, Rust stream borrow c…
Browse files Browse the repository at this point in the history
…hecks (#515)

## What is the goal of this PR?

We improve the UX of Driver usage in several ways:

- [Rust/Java/Python] Fix the formatting of error messages received from
the server during a transaction.
- [Java] Fix native error message formatting.
- [Java/Python] Add the ability to enable Rust logging over FFI
(resolves #482).
- The granularity of the logs is controlled by the
`TYPEDB_DRIVER_LOG_LEVEL` environment variable, which can be set to any
value of `error` (default), `warn`, `info`, `debug`, or `trace`, in
order of increasing verbosity. Setting the value to
`"typedb_driver=info"` will show only the typedb-driver messages.
- More advanced syntax is described in [the env_logger
documentation](https://docs.rs/env_logger/latest/env_logger/index.html#enabling-logging)
- [Rust] Network streams now borrow the transaction, so that the
transaction can't be mistakenly dropped. (resolves #449)
  • Loading branch information
dmitrii-ubskii authored Nov 3, 2023
1 parent fc97c57 commit 96be0f6
Show file tree
Hide file tree
Showing 34 changed files with 699 additions and 663 deletions.
1 change: 1 addition & 0 deletions c/BUILD
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,7 @@ rust_static_library(

"@crates//:chrono",
"@crates//:itertools",
"@crates//:env_logger",
"@crates//:log",
],
)
Expand Down
11 changes: 10 additions & 1 deletion c/src/error.rs
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,8 @@

use std::{cell::RefCell, ffi::c_char, ptr::null_mut};

use log::trace;
use env_logger::Env;
use log::{trace, warn};
use typedb_driver::{Error, Result};

use super::memory::{free, release_optional, release_string};
Expand All @@ -30,6 +31,14 @@ thread_local! {
static LAST_ERROR: RefCell<Option<Error>> = RefCell::new(None);
}

#[no_mangle]
pub extern "C" fn init_logging() {
const ENV_VAR: &str = "TYPEDB_DRIVER_LOG_LEVEL";
if let Err(err) = env_logger::try_init_from_env(Env::new().filter(ENV_VAR)) {
warn!("{err}");
}
}

fn ok_record<T>(result: Result<T>) -> Option<T> {
match result {
Ok(value) => Some(value),
Expand Down
4 changes: 3 additions & 1 deletion c/swig/typedb_driver_java.swg
Original file line number Diff line number Diff line change
Expand Up @@ -59,7 +59,7 @@

public static class Unchecked extends RuntimeException {
Unchecked(Error e) {
super(e);
super(e.getMessage());
}
}
%}
Expand All @@ -83,6 +83,8 @@
}
}

%nojavaexception init_logging;

/* simple getters do not throw */
%nojavaexception options_new;
%nojavaexception options_get_infer;
Expand Down
2 changes: 2 additions & 0 deletions c/swig/typedb_driver_python.swg
Original file line number Diff line number Diff line change
Expand Up @@ -98,6 +98,8 @@ static PyObject* PyExc_TypeDBDriverError;

%pythoncode %{
TypeDBDriverExceptionNative = native_driver_python.TypeDBDriverExceptionNative

native_driver_python.init_logging()
%}

/* ValueType* needs special handling */
Expand Down
2 changes: 1 addition & 1 deletion dependencies/vaticle/repositories.bzl
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@ def vaticle_dependencies():
git_repository(
name = "vaticle_dependencies",
remote = "https://github.com/vaticle/dependencies",
commit = "67a0602be830227247d31bb881029d752f05f9bb", # sync-marker: do not remove this comment, this is used for sync-dependencies by @vaticle_dependencies
commit = "d75f30de7902892b1d45015aefd20f06048b0458", # sync-marker: do not remove this comment, this is used for sync-dependencies by @vaticle_dependencies
)

def vaticle_typedb_common():
Expand Down
3 changes: 3 additions & 0 deletions java/common/NativeObject.java
Original file line number Diff line number Diff line change
Expand Up @@ -24,9 +24,12 @@
import com.vaticle.typedb.driver.common.exception.ErrorMessage;
import com.vaticle.typedb.driver.common.exception.TypeDBDriverException;

import static com.vaticle.typedb.driver.jni.typedb_driver.init_logging;

public abstract class NativeObject<T> {
static {
Loader.loadNativeLibraries();
init_logging();
}

public final T nativeObject;
Expand Down
8 changes: 0 additions & 8 deletions java/common/exception/TypeDBDriverException.java
Original file line number Diff line number Diff line change
Expand Up @@ -28,9 +28,6 @@
*/
public class TypeDBDriverException extends RuntimeException {

@Nullable
private final com.vaticle.typedb.driver.jni.Error nativeError;

@Nullable
private final ErrorMessage errorMessage;

Expand All @@ -40,7 +37,6 @@ public class TypeDBDriverException extends RuntimeException {
public TypeDBDriverException(ErrorMessage error, Object... parameters) {
super(error.message(parameters));
assert !getMessage().contains("%s");
this.nativeError = null;
this.errorMessage = error;
}

Expand All @@ -49,7 +45,6 @@ public TypeDBDriverException(ErrorMessage error, Object... parameters) {
*/
public TypeDBDriverException(String message, Throwable cause) {
super(message, cause);
this.nativeError = null;
this.errorMessage = null;
}

Expand All @@ -59,8 +54,6 @@ public TypeDBDriverException(String message, Throwable cause) {
public TypeDBDriverException(RuntimeException error) {
super(error.getMessage());
assert !getMessage().contains("%s");
if (error.getCause() instanceof com.vaticle.typedb.driver.jni.Error) this.nativeError = (com.vaticle.typedb.driver.jni.Error)error.getCause();
else this.nativeError = null;
this.errorMessage = null;
}

Expand All @@ -70,7 +63,6 @@ public TypeDBDriverException(RuntimeException error) {
public TypeDBDriverException(com.vaticle.typedb.driver.jni.Error error) {
super(error.getMessage());
assert !getMessage().contains("%s");
this.nativeError = error;
this.errorMessage = null;
}

Expand Down
4 changes: 2 additions & 2 deletions rust/docs/concept/ConceptManager.adoc
Original file line number Diff line number Diff line change
Expand Up @@ -336,7 +336,7 @@ transaction.concepts().get_relation_type(label).resolve()
----
pub fn get_schema_exceptions(
&self
) -> Result<impl Stream<Item = Result<SchemaException>>>
) -> Result<impl Stream<Item = Result<SchemaException>> + 'tx>
----
Retrieves a list of all schema exceptions for the current transaction.
Expand All @@ -345,7 +345,7 @@ Retrieves a list of all schema exceptions for the current transaction.
.Returns
[source,rust]
----
Result<impl Stream<Item = Result<SchemaException>>>
Result<impl Stream<Item = Result<SchemaException>> + 'tx>
----
[caption=""]
Expand Down
48 changes: 24 additions & 24 deletions rust/docs/data/Attribute.adoc
Original file line number Diff line number Diff line change
Expand Up @@ -33,8 +33,8 @@ a| `value` a| `Value` a| The value which this Attribute instance holds.
[source,rust]
----
fn delete<'tx>(
&'tx self,
transaction: &'tx Transaction<'tx>
&self,
transaction: &'tx Transaction<'_>
) -> BoxPromise<'tx, Result>
----
Expand All @@ -52,12 +52,12 @@ BoxPromise<'tx, Result>
[source,rust]
----
fn get_has(
fn get_has<'tx>(
&self,
transaction: &Transaction<'_>,
transaction: &'tx Transaction<'_>,
attribute_types: Vec<AttributeType>,
annotations: Vec<Annotation>
) -> Result<BoxStream<'_, Result<Attribute>>>
) -> Result<BoxStream<'tx, Result<Attribute>>>
----
<<#_trait_ThingAPI_method_get_has,Read more>>
Expand All @@ -66,19 +66,19 @@ fn get_has(
.Returns
[source,rust]
----
Result<BoxStream<'_, Result<Attribute>>>
Result<BoxStream<'tx, Result<Attribute>>>
----
[#_struct_Attribute_method_get_owners]
==== get_owners
[source,rust]
----
fn get_owners(
fn get_owners<'tx>(
&self,
transaction: &Transaction<'_>,
transaction: &'tx Transaction<'_>,
thing_type: Option<ThingType>
) -> Result<BoxStream<'_, Result<Thing>>>
) -> Result<BoxStream<'tx, Result<Thing>>>
----
<<#_trait_AttributeAPI_method_get_owners,Read more>>
Expand All @@ -87,18 +87,18 @@ fn get_owners(
.Returns
[source,rust]
----
Result<BoxStream<'_, Result<Thing>>>
Result<BoxStream<'tx, Result<Thing>>>
----
[#_struct_Attribute_method_get_playing]
==== get_playing
[source,rust]
----
fn get_playing(
fn get_playing<'tx>(
&self,
transaction: &Transaction<'_>
) -> Result<BoxStream<'_, Result<RoleType>>>
transaction: &'tx Transaction<'_>
) -> Result<BoxStream<'tx, Result<RoleType>>>
----
<<#_trait_ThingAPI_method_get_playing,Read more>>
Expand All @@ -107,19 +107,19 @@ fn get_playing(
.Returns
[source,rust]
----
Result<BoxStream<'_, Result<RoleType>>>
Result<BoxStream<'tx, Result<RoleType>>>
----
[#_struct_Attribute_method_get_relations]
==== get_relations
[source,rust]
----
fn get_relations(
fn get_relations<'tx>(
&self,
transaction: &Transaction<'_>,
transaction: &'tx Transaction<'_>,
role_types: Vec<RoleType>
) -> Result<BoxStream<'_, Result<Relation>>>
) -> Result<BoxStream<'tx, Result<Relation>>>
----
<<#_trait_ThingAPI_method_get_relations,Read more>>
Expand All @@ -128,7 +128,7 @@ fn get_relations(
.Returns
[source,rust]
----
Result<BoxStream<'_, Result<Relation>>>
Result<BoxStream<'tx, Result<Relation>>>
----
[#_struct_Attribute_tymethod_iid]
Expand All @@ -154,8 +154,8 @@ fn iid(&self) -> &IID
[source,rust]
----
fn is_deleted<'tx>(
&'tx self,
transaction: &'tx Transaction<'tx>
&self,
transaction: &'tx Transaction<'_>
) -> BoxPromise<'tx, Result<bool>>
----
Expand Down Expand Up @@ -191,8 +191,8 @@ bool
[source,rust]
----
fn set_has<'tx>(
&'tx self,
transaction: &'tx Transaction<'tx>,
&self,
transaction: &'tx Transaction<'_>,
attribute: Attribute
) -> BoxPromise<'tx, Result>
----
Expand All @@ -212,8 +212,8 @@ BoxPromise<'tx, Result>
[source,rust]
----
fn unset_has<'tx>(
&'tx self,
transaction: &'tx Transaction<'tx>,
&self,
transaction: &'tx Transaction<'_>,
attribute: Attribute
) -> BoxPromise<'tx, Result>
----
Expand Down
40 changes: 20 additions & 20 deletions rust/docs/data/Entity.adoc
Original file line number Diff line number Diff line change
Expand Up @@ -34,8 +34,8 @@ a| `type_` a| `EntityType` a| The type which this Entity belongs to
[source,rust]
----
fn delete<'tx>(
&'tx self,
transaction: &'tx Transaction<'tx>
&self,
transaction: &'tx Transaction<'_>
) -> BoxPromise<'tx, Result>
----
Expand All @@ -53,12 +53,12 @@ BoxPromise<'tx, Result>
[source,rust]
----
fn get_has(
fn get_has<'tx>(
&self,
transaction: &Transaction<'_>,
transaction: &'tx Transaction<'_>,
attribute_types: Vec<AttributeType>,
annotations: Vec<Annotation>
) -> Result<BoxStream<'_, Result<Attribute>>>
) -> Result<BoxStream<'tx, Result<Attribute>>>
----
<<#_trait_ThingAPI_method_get_has,Read more>>
Expand All @@ -67,18 +67,18 @@ fn get_has(
.Returns
[source,rust]
----
Result<BoxStream<'_, Result<Attribute>>>
Result<BoxStream<'tx, Result<Attribute>>>
----
[#_struct_Entity_method_get_playing]
==== get_playing
[source,rust]
----
fn get_playing(
fn get_playing<'tx>(
&self,
transaction: &Transaction<'_>
) -> Result<BoxStream<'_, Result<RoleType>>>
transaction: &'tx Transaction<'_>
) -> Result<BoxStream<'tx, Result<RoleType>>>
----
<<#_trait_ThingAPI_method_get_playing,Read more>>
Expand All @@ -87,19 +87,19 @@ fn get_playing(
.Returns
[source,rust]
----
Result<BoxStream<'_, Result<RoleType>>>
Result<BoxStream<'tx, Result<RoleType>>>
----
[#_struct_Entity_method_get_relations]
==== get_relations
[source,rust]
----
fn get_relations(
fn get_relations<'tx>(
&self,
transaction: &Transaction<'_>,
transaction: &'tx Transaction<'_>,
role_types: Vec<RoleType>
) -> Result<BoxStream<'_, Result<Relation>>>
) -> Result<BoxStream<'tx, Result<Relation>>>
----
<<#_trait_ThingAPI_method_get_relations,Read more>>
Expand All @@ -108,7 +108,7 @@ fn get_relations(
.Returns
[source,rust]
----
Result<BoxStream<'_, Result<Relation>>>
Result<BoxStream<'tx, Result<Relation>>>
----
[#_struct_Entity_tymethod_iid]
Expand All @@ -134,8 +134,8 @@ fn iid(&self) -> &IID
[source,rust]
----
fn is_deleted<'tx>(
&'tx self,
transaction: &'tx Transaction<'tx>
&self,
transaction: &'tx Transaction<'_>
) -> BoxPromise<'tx, Result<bool>>
----
Expand Down Expand Up @@ -171,8 +171,8 @@ bool
[source,rust]
----
fn set_has<'tx>(
&'tx self,
transaction: &'tx Transaction<'tx>,
&self,
transaction: &'tx Transaction<'_>,
attribute: Attribute
) -> BoxPromise<'tx, Result>
----
Expand All @@ -192,8 +192,8 @@ BoxPromise<'tx, Result>
[source,rust]
----
fn unset_has<'tx>(
&'tx self,
transaction: &'tx Transaction<'tx>,
&self,
transaction: &'tx Transaction<'_>,
attribute: Attribute
) -> BoxPromise<'tx, Result>
----
Expand Down
Loading

0 comments on commit 96be0f6

Please sign in to comment.