CASSGO-38 Marshal error messages enhancement #1838

tengu-alt · 2024-10-30T12:01:29Z

This PR provides marshal/unmarshal error messages enhancement.
According to #671 errors were updated by the supported types list.

worryg0d · 2024-10-31T08:22:34Z

marshal.go

@@ -333,7 +333,7 @@ func marshalVarchar(info TypeInfo, value interface{}) ([]byte, error) {
 	case k == reflect.Slice && t.Elem().Kind() == reflect.Uint8:
 		return rv.Bytes(), nil
 	}
-	return nil, marshalErrorf("can not marshal %T into %s", value, info)
+	return nil, marshalErrorf("can not marshal %T into %s. Accepted types: Marshaler, string, []byte, unsetColumn.", value, info)


unsetColumn is not an exported type, so the driver doesn't have to expose it to the user. There is a UnsetValue global var of type unsetColumn which is meant to be used to ignore writing. You can find it here.

Thank you, refactored

I meant to change unsetColumn to UnsetValue, not to delete it at all

I see, fixed.

worryg0d · 2024-10-31T08:34:14Z

marshal.go

@@ -2280,7 +2280,7 @@ func marshalUDT(info TypeInfo, value interface{}) ([]byte, error) {
 	}

 	if k.Kind() != reflect.Struct || !k.IsValid() {
-		return nil, marshalErrorf("cannot marshal %T into %s", value, info)
+		return nil, marshalErrorf("cannot marshal %T into %s. Accepted types: Marshaler, unsetColumn, UDTMarshaler, UDTMarshaler, map[string]interface{}, struct.", value, info)


Here UDTMarshaler is duplicated

ribaraka · 2024-11-22T11:57:42Z

https://issues.apache.org/jira/browse/CASSGO-38

joao-r-reis · 2024-11-22T14:09:02Z

marshal.go

@@ -2208,7 +2210,7 @@ func unmarshalTuple(info TypeInfo, data []byte, value interface{}) error {
 		return nil
 	}

-	return unmarshalErrorf("cannot unmarshal %s into %T", info, value)
+	return unmarshalErrorf("cannot unmarshal %s into %T. Accepted types: struct, []interface{}, array, slice.", info, value)


Unmarshaler is also accepted I think. Also the accepted types other than []interface{} have to be a pointer if I'm reading the code correctly (so *struct, *array, *slice)

Good catch! Thank you.

marshal.go

joao-r-reis · 2024-11-22T14:19:45Z

marshal.go

@@ -1706,7 +1708,7 @@ func unmarshalList(info TypeInfo, data []byte, value interface{}) error {
 		}
 		return nil
 	}
-	return unmarshalErrorf("can not unmarshal %s into %T", info, value)
+	return unmarshalErrorf("can not unmarshal %s into %T. Accepted types: slice, array.", info, value)


I think it has to be *slice, *array

joao-r-reis · 2024-11-22T14:24:45Z

marshal.go

@@ -2392,7 +2394,7 @@ func unmarshalUDT(info TypeInfo, data []byte, value interface{}) error {
 	}
 	k := rv.Elem()
 	if k.Kind() != reflect.Struct || !k.IsValid() {
-		return unmarshalErrorf("cannot unmarshal %s into %T", info, value)
+		return unmarshalErrorf("cannot unmarshal %s into %T. Accepted types: Unmarshaler, UDTUnmarshaler, *map[string]interface{}, struct.", info, value)


jameshartig · 2025-01-25T20:52:40Z

It's not very common in Go for the error to explicitly provide the supported types but I can see how that's useful. Ideally a developer would be able to know the supported types before encountering an error. In #671 what made you think that timestamp would support an int? Was it just common sense or did you read something indicating as much?

I'd like to see improvements to the docs and a general improvement in the supported types. That said, nothing is wrong with this PR.

tengu-alt · 2025-01-28T10:01:34Z

In #671 what made you think that timestamp would support an int? Was it just common sense or did you read something indicating as much?

I don't clearly understand your question. The driver already supports Timestamp marshal into int64, as said in the codebase and documentation.

jameshartig · 2025-01-28T15:48:47Z

I don't clearly understand your question.

In #671 you ran into:

2016/03/08 08:35:36 can not marshal int into timestamp

Which meant you sent an int, that should be supported. I'm trying to understand how we can improve the documentation or API to make it more obvious which types are supported that doesn't involve someone having to parse an error after already writing code which assumed a certain type.

tengu-alt · 2025-01-29T09:46:08Z

In #671 you ran into:

That was not me who ran into the error, the #671 was created by another person.

As Cassandra doc says: timestamp is encoded as int64. The default int in GO is a 32bit integer, so it is not supported.

IMHO the documentation in the marshal.go is pretty obvious, and it appears when you call the func Marshal().

jameshartig · 2025-01-29T14:47:39Z

That was not me who ran into the error, the #671 was created by another person.

Oh apologies for the confusion.

The default int in GO is a 32bit integer, so it is not supported.

Only on 32-bit systems, which are increasingly rare. We should be fine marshalling into Timestamp but maybe we just couldn't safely unmarshal

Enhancement for marshal/unmarshal error messages. Errors were updated by the supported types list. patch by Oleksandr Luzhniy; reviewed by João Reis, James Hartig, Bohdan Siryk, for CASSGO-38

tengu-alt · 2025-01-31T10:53:21Z

@joao-r-reis I have updated the Changelog.md, should it be merged?

tengu-alt force-pushed the marshal-errors-enhancement branch from bb0dab6 to c3dcd98 Compare October 30, 2024 12:02

worryg0d reviewed Oct 31, 2024

View reviewed changes

tengu-alt force-pushed the marshal-errors-enhancement branch 3 times, most recently from f4a0127 to dbec58a Compare October 31, 2024 12:50

tengu-alt force-pushed the marshal-errors-enhancement branch from dbec58a to d6550c9 Compare November 20, 2024 11:10

joao-r-reis reviewed Nov 22, 2024

View reviewed changes

joao-r-reis changed the title ~~Marshal error messages enhancement~~ CASSGO-38 Marshal error messages enhancement Nov 22, 2024

tengu-alt force-pushed the marshal-errors-enhancement branch from d6550c9 to 74254c8 Compare November 22, 2024 14:49

joao-r-reis approved these changes Nov 22, 2024

View reviewed changes

jameshartig approved these changes Jan 29, 2025

View reviewed changes

tengu-alt force-pushed the marshal-errors-enhancement branch from 74254c8 to 87f1776 Compare January 31, 2025 10:46

Improve error messages for marshalling

bc810b7

Enhancement for marshal/unmarshal error messages. Errors were updated by the supported types list. patch by Oleksandr Luzhniy; reviewed by João Reis, James Hartig, Bohdan Siryk, for CASSGO-38

tengu-alt force-pushed the marshal-errors-enhancement branch from 87f1776 to bc810b7 Compare January 31, 2025 10:49

joao-r-reis merged commit 16366d4 into apache:trunk Jan 31, 2025
16 checks passed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

CASSGO-38 Marshal error messages enhancement #1838

CASSGO-38 Marshal error messages enhancement #1838

tengu-alt commented Oct 30, 2024

worryg0d Oct 31, 2024

tengu-alt Oct 31, 2024

worryg0d Oct 31, 2024

tengu-alt Oct 31, 2024

worryg0d Oct 31, 2024

tengu-alt Oct 31, 2024

ribaraka commented Nov 22, 2024

joao-r-reis Nov 22, 2024

tengu-alt Nov 22, 2024

joao-r-reis Nov 22, 2024

tengu-alt Nov 22, 2024

joao-r-reis Nov 22, 2024

tengu-alt Nov 22, 2024

jameshartig commented Jan 25, 2025

tengu-alt commented Jan 28, 2025 •

edited

Loading

jameshartig commented Jan 28, 2025

tengu-alt commented Jan 29, 2025 •

edited

Loading

jameshartig commented Jan 29, 2025 •

edited

Loading

tengu-alt commented Jan 31, 2025

CASSGO-38 Marshal error messages enhancement #1838

CASSGO-38 Marshal error messages enhancement #1838

Conversation

tengu-alt commented Oct 30, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ribaraka commented Nov 22, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jameshartig commented Jan 25, 2025

tengu-alt commented Jan 28, 2025 • edited Loading

jameshartig commented Jan 28, 2025

tengu-alt commented Jan 29, 2025 • edited Loading

jameshartig commented Jan 29, 2025 • edited Loading

tengu-alt commented Jan 31, 2025

tengu-alt commented Jan 28, 2025 •

edited

Loading

tengu-alt commented Jan 29, 2025 •

edited

Loading

jameshartig commented Jan 29, 2025 •

edited

Loading