Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

TileDB Error's are not actionable #30

Closed
jakebolewski opened this issue Jun 15, 2017 · 14 comments
Closed

TileDB Error's are not actionable #30

jakebolewski opened this issue Jun 15, 2017 · 14 comments

Comments

@jakebolewski
Copy link
Contributor

Currently when using the C-API there is no way to tell what the error was (class) without parsing the error string.

@stavrospapadopoulos
Copy link
Member

In addition to refactoring error handling, all C API functions should return a tiledb_err_t object instead of int. We can then add extra functions to act on tiledb_err_t objects.

@jakebolewski
Copy link
Contributor Author

jakebolewski commented Jun 23, 2017

I don't know any C Api that does it this way? Do you have an example.

I was thinking of sticking to Posix semantics here and for more fine grained error handling implementing it in a way similar to libgit2.

Ex:
tiledb_error_last() -> tiledb_error_t
tiledb_error_message(tiledb_error_t*)
tiledb_error_class(tiledb_error_t*) -> Error Class Enum
tiledb_error_code(tiledb_error_t*) -> Error Code Enum (per class)

@jakebolewski
Copy link
Contributor Author

Wrong link, updated.

@stavrospapadopoulos
Copy link
Member

I like it, let's do that.

@jakebolewski
Copy link
Contributor Author

jakebolewski commented Jul 5, 2017

This issue needs a bit more work in order to be closed:

  • define a c api for getting the message, error class, and error code from an error object
  • a thread safe stack for consolidating all error's returned from all threads per operation
  • a c api for iterating over possibly all error's returned after a given api call

@stavrospapadopoulos
Copy link
Member

Currently we save the status errors in a (created) context. Some C API functions do not take the context as input (e.g., all tiledb_config functions, in the future all tiledb_array_schema functions, etc.) . How do we get the status error in these cases?

@jakebolewski
Copy link
Contributor Author

jakebolewski commented Jul 10, 2017 via email

@stavrospapadopoulos
Copy link
Member

This means that every single API function will take the context as input. Are you ok with this?

Moreover, please check how to handle the error of initializing a storage manager in tiledb_ctx_create. If this init fails, the user cannot capture it (it will just get a nullptr for the returned ctx).

@stavrospapadopoulos
Copy link
Member

Taking another look, I think that we should have a uniform approach. Currently, ctx is attached to only some of the functions. Perhaps we should have it as input everywhere?

@jakebolewski
Copy link
Contributor Author

I guess I am arguing for consistency in threading the ctx. Otherwise the only way to do this (that I see) is to have a global error state. The only way to disambiguate which from which context an error was thrown would be to provide it so I don't see what you gain.

Maybe there is a cleaner design, but I think associating error's with a given context is the simplest.

@jakebolewski
Copy link
Contributor Author

You can guarantee (well pretty much) that construction of a valid Storage Manager object will succeed. If during initialization (init) of the storage manager an error is thrown you can still associate it with a valid ctx object, so I don't see the problem.

@stavrospapadopoulos
Copy link
Member

In the rest of the API, when an error occurs, a TILEDB_ERR is returned, whereas here we return null or a valid context. So how does the user gets notified about the error? I'll check on wherher initing the storage manager can always succeed.

@jakebolewski
Copy link
Contributor Author

jakebolewski commented Jul 10, 2017 via email

@stavrospapadopoulos
Copy link
Member

Hehe sure, but then it is not consistent with all the other 'tiledb_*_create' functions. I may have to change all of them, which is not too bad.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants