View Issue Details

Jump to NotesJump to History
IDProjectCategoryView StatusDate SubmittedLast Update
0007662Talerlibeufin-bankpublic2023-02-07 15:342024-07-28 21:52
Reportersebasjm Assigned ToAntoine A  
PrioritynormalSeveritytweakReproducibilityalways
Status feedbackResolutionopen 
Target Versionpost-1.0 
Summary0007662: error does not provide the field where the validation is made
DescriptionSome validation show the field where the error is, but for the username looks like not. It will be nice to have the some kind of error that help fixing the problem.

Also, I couldn't find documentation of this validations.

curl 'https://sandbox-libeufin.taler.ar/demobanks/default/circuit-api/accounts' -d '{"username":"1","password":"1","name":"the name","contact_data": {},"cashout_address":"payto://iban/DE123123"}' -v

  "error" : {
    "type" : "util-error",
    "description" : "Invalid resource name. The first character must be a lowercase letter, and all following characters (except for the last character) must be a dash, lowercase letter, or digit. The last character must be a lowercase letter or digit."
  }
TagsNo tags attached.

Activities

Antoine A

2023-12-02 03:42

developer   ~0020681

`kotlinx.serialization` is the official Kotlin serialization library we use for JSON. It loses parsing context when the parsing exception doesn't come from the library, this is a [problem](https://github.com/Kotlin/kotlinx.serialization/issues/2165) [known](https://github.com/Kotlin/kotlinx.serialization/issues/316) and they don't seem to be in a hurry to fix it as the first issue is from 2018.
If we don't want to wait for them to fix this properly, we can switch to another library like [moshi](https://github.com/square/moshi) which handles this [better](https://github.com/square/moshi#fails-gracefully). It's not trivial, and switching to an unofficial library is not an easy decision to make, even if Square is a trusted open source contributor.

Christian Grothoff

2023-12-02 08:40

manager   ~0020682

I'm confused, what does this have to do with the JSON parser? The JSON seems well-formed here. I assume *we* are somewhere throwing an exception in the user name validation. That's fine, but as *we* throw that exception, we should be able to include the field name in our exception object and/or in our exception error message, right?

Antoine A

2023-12-02 08:52

developer   ~0020683

In our code, we have an `IbanPayto` type and code that tells `kotlinx.serialization` how to parse it from a JSON string. When this function is called, we do not know where we are in the JSON, this context information is not given to us, so when we throw an exception, we can't provide that context. Then, when `kotlinx.serialization` caught our exception, because the error is not thrown by the library itself, they choose not to provide the JSON error path but simply propagate the error. This is why every time an error is caused by our custom types (for example a JSON string that is a TalerAmount or an IbanPayto), we lose the error path.

Christian Grothoff

2023-12-02 08:58

manager   ~0020684

Eh, the error reported in this bug seems clearly about the username, not the payto://-URI. And in both cases, you can still report that this was about a payto://-URI or about a username, which would be much clearer than the current message that doesn't mention the data type involved in the failure at all.

Antoine A

2023-12-02 09:09

developer   ~0020685

With libeufin-bank the request from the issue works whithout payto uri:
curl 'http://127.0.0.1:8080/accounts' -H 'Content-Type: application/json' -d '{"username":"1","password":"1","name":"the name","contact_data": {}}' -v
{
    "internal_payto_uri": "payto://iban/DE3192435642004"
}

But I do not consider this error fixed because of this new error:
curl 'http://127.0.0.1:8080/accounts' -H 'Content-Type: application/json' -d '{"username":"1","password":"1","name":"the name","contact_data": {},ca
sshout_payto_uri":"payto://iban/DE12312"}' -v
{
    "code": 22,
    "hint": "Failed to convert request body to class tech.libeufin.bank.RegisterAccountRequest",
    "detail": "Iban malformed, modulo is 59 expected 1"
}

Christian Grothoff

2023-12-02 11:01

manager   ~0020686

Eh, but that seems like a perfectly good error message to give when the IBAN checksum is invalid. What's wrong with that?

sebasjm

2023-12-02 15:57

developer   ~0020709

from my POV this is only for development/testing when the API change, not something the user should see and something I can keep up if we have the proper API change notifications. It would be nice to have since I can use this error message to fix without asking.

It also happen with the `update currency conversion` where the error was "wrong currency" but i didn't know which field. So a good generic solution will be good.

That said, I'm ok with keeping this post 1.0 if this requires a lib change.

Antoine A

2024-07-01 11:54

developer   ~0022774

Pending on kotlinx.serialization to supports this

Issue History

Date Modified Username Field Change
2023-02-07 15:34 sebasjm New Issue
2023-02-07 15:34 sebasjm Status new => assigned
2023-02-07 15:34 sebasjm Assigned To => MS
2023-04-11 11:30 MS Status assigned => acknowledged
2023-04-13 20:26 Florian Dold Category sandbox => libeufin sandbox
2023-04-13 20:26 Florian Dold Project libeufin => Taler
2023-04-13 20:26 Florian Dold Category libeufin sandbox => General
2023-04-13 21:44 Florian Dold Category General => libeufin-sandbox
2023-04-13 21:44 Florian Dold Product Version Git master =>
2023-06-29 12:03 MS Target Version 0.9.3 => 0.9.4
2023-09-03 18:21 Christian Grothoff Assigned To MS => Antoine A
2023-09-23 15:26 Christian Grothoff Category libeufin-sandbox => libeufin-bank
2023-12-02 03:42 Antoine A Note Added: 0020681
2023-12-02 08:40 Christian Grothoff Note Added: 0020682
2023-12-02 08:52 Antoine A Note Added: 0020683
2023-12-02 08:58 Christian Grothoff Note Added: 0020684
2023-12-02 09:09 Antoine A Note Added: 0020685
2023-12-02 11:01 Christian Grothoff Note Added: 0020686
2023-12-02 15:57 sebasjm Note Added: 0020709
2023-12-22 14:55 Christian Grothoff Target Version 0.9.4 => post-1.0
2023-12-24 06:26 Christian Grothoff Status acknowledged => assigned
2024-07-01 11:54 Antoine A Status assigned => feedback
2024-07-01 11:54 Antoine A Note Added: 0022774
2024-07-28 21:52 Christian Grothoff Severity minor => tweak