scylladb / scylladb/cpp-rs-driver

Handle failures in `void` functions

Open
#453 0 comments 0 reactions 0 assignees View on GitHub

Nobody has claimed this yet.

Dominant language
C++
Stars
26
Forks
19
Avg merge
1d 21h
Merged PRs (30d)
7

Description

Background

cass_cluster_set_application_name_n

I wonder if this is the proper way of handling this. It will fail mostly silently - only signal will be an entry in the logs.
Perhaps a panic is warranted here?

Originally posted by @Lorak-mmk in https://github.com/scylladb/cpp-rs-driver/pull/448#discussion_r3258169929

Problem

There are quite a lot of public API functions that return void, yet still may fail. Currently, upon error we issue an error! log message, followed by an early return.

@lorak-mmk correctly noted that this may easily go unnoticed, causing silent bugs. To combat this, he proposed panicking in critical conditions.

Still needs to be settled

It's up to further discussion which error conditions shall be considered critical (and warrant a panic) and which not (so the error log is enough).

For now, we have the following examples of functions falling into each of these categories:

Critical errors
  1. cass_cluster_set_credentials - if it fails, we will likely be unable to connect to the cluster and execute any statements.
Noncritical errors
  1. cass_cluster_set_application_{name,version} - failure to advertise the desired name of the application for the cluster is by far not something that stop driver from doing its work.

Contributor guide

No contributing guide indexed for this repository

First steps

  1. Read the whole issue, then the project's contributing guide.
  2. Comment on the issue to say you are picking it up — it saves two people doing the same work.
  3. Fork the repository and make your change on a branch.
  4. Open a pull request that references the issue number.

Research direction

Start by reviewing the public API functions that return void, especially cass_cluster_set_credentials and cass_cluster_set_application_{name,version}, along with their current error logging and early returns. The issue is done when the project has decided which failure conditions are critical and the corresponding panic-versus-log behavior is consistently implemented.

Written by the indexing model from the issue text.

Assessment

Tech stack
cpp, rust
Domain
api, backend-api-design
Issue type
Feature
Difficulty
5/5
Estimated time
Over a week
Activity status
Quiet
Clarity
Needs clarification
Newbie friendliness
35/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.