DiceDB should not return golang errors to users

[JyotinderSingh]

We return golang errors to users in some of the error paths in our code, largely in functions defined inside eval.go.

This is undesirable. We should only be returning custom error messages that are designed by us to the users, instead of exposing the underlying language details.

As a part of this issue, you will need to identify places where we might be returning language specific errors to the users, and replace them with custom dicedb error messages.

[benjaminmishra]

Hi @JyotinderSingh would love to pick this up.

[BadriVishalPadhy]

Hey @JyotinderSingh i would love to resolve this issue

[JyotinderSingh]

Hi @JyotinderSingh would love to pick this up.

Assigned.

[pree04]

@JyotinderSingh Please assign me some issue to work on . I want to contribute .

[benjaminmishra]

Hi @JyotinderSingh , Just to make sure I understand this correctly, follwing is a code snippet from the evalRPUSH in eval.go for your reference. Here I noticed that we are using return clientio.Encode(err, false) instead of using diceerror. Would you say this that this one example of what we need to correct ?

if err := object.AssertType(obj.TypeEncoding, object.ObjTypeByteList); err != nil {
	return clientio.Encode(err, false)
}

if err := object.AssertEncoding(obj.TypeEncoding, object.ObjEncodingDeque); err != nil {
	return clientio.Encode(err, false)
}

[JyotinderSingh]

Hi @JyotinderSingh , Just to make sure I understand this correctly, follwing is a code snippet from the evalRPUSH in eval.go for your reference. Here I noticed that we are using return clientio.Encode(err, false) instead of using diceerror. Would you say this that this one example of what we need to correct ?

if err := object.AssertType(obj.TypeEncoding, object.ObjTypeByteList); err != nil {

	return clientio.Encode(err, false)

}



if err := object.AssertEncoding(obj.TypeEncoding, object.ObjEncodingDeque); err != nil {

	return clientio.Encode(err, false)

}

Not exactly, the assertType method internally returns a custom error - which is okay.

We are looking to fix instances where we may be seeing golang throw an error and returning the same error back to the user. For instance, say somewhere we try to convert a random string to a number using the ParseFloat API and the operation fails, we should return golang's error message but rather our own (ideally similar to what redis returns)

[JyotinderSingh]

Hi @JyotinderSingh , Just to make sure I understand this correctly, follwing is a code snippet from the evalRPUSH in eval.go for your reference. Here I noticed that we are using return clientio.Encode(err, false) instead of using diceerror. Would you say this that this one example of what we need to correct ?

if err := object.AssertType(obj.TypeEncoding, object.ObjTypeByteList); err != nil {

	return clientio.Encode(err, false)

}



if err := object.AssertEncoding(obj.TypeEncoding, object.ObjEncodingDeque); err != nil {

	return clientio.Encode(err, false)

}

Not exactly, the assertType method internally returns a custom error - which is okay.

We are looking to fix instances where we may be seeing golang throw an error and returning the same error back to the user. For instance, say somewhere we try to convert a random string to a number using the ParseFloat API and the operation fails, we should not return golang's error message but rather our own (ideally similar to what redis returns)