View Issue Details
| ID | Project | Category | View Status | Date Submitted | Last Update |
|---|---|---|---|---|---|
| 0007662 | Taler | libeufin-bank | public | 2023-02-07 14:34 | 2026-08-29 19:10 |
| Reporter | sebasjm | Assigned To | Resolution deadline | ||
| Implementer | Reviewer | ||||
| Release signed by | Deployed by | ||||
| Verifier | Communicator | ||||
| Data protection authority notification required | No | ||||
| CSIRT notification required | No | ||||
| Estimated effort (hours) | Actual effort (hours) | 0 | |||
| Fast-track | No | ||||
| Work package | none | ||||
| Priority | normal | Severity | tweak | Reproducibility | always |
| Status | acknowledged | Resolution | open | ||
| Target Version | post-1.0 | ||||
| Summary | 0007662: error does not provide the field where the validation is made | ||||
| Description | Some 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." } | ||||
| Tags | No tags attached. | ||||
| `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. | |
| 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? | |
| 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. | |
| 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. | |
| 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" } | |
| Eh, but that seems like a perfectly good error message to give when the IBAN checksum is invalid. What's wrong with that? | |
| 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. | |
| Pending on kotlinx.serialization to supports this | |
| Date Modified | Username | Field | Change |
|---|---|---|---|
| 2023-02-07 14:34 | sebasjm | New Issue | |
| 2023-02-07 14:34 | sebasjm | Status | new => assigned |
| 2023-02-07 14:34 | sebasjm | Assigned To | => MS |
| 2023-04-11 09:30 | MS | Status | assigned => acknowledged |
| 2023-04-13 18:26 | Florian Dold | Category | sandbox => libeufin sandbox |
| 2023-04-13 18:26 | Florian Dold | Project | libeufin => Taler |
| 2023-04-13 18:26 | Florian Dold | Category | libeufin sandbox => General |
| 2023-04-13 19:44 | Florian Dold | Category | General => libeufin-sandbox |
| 2023-04-13 19:44 | Florian Dold | Product Version | Git master => |
| 2023-06-29 10:03 | MS | Target Version | 0.9.3 => 0.9.4 |
| 2023-09-03 16:21 | Christian Grothoff | Assigned To | MS => Antoine A |
| 2023-09-23 13:26 | Christian Grothoff | Category | libeufin-sandbox => libeufin-bank |
| 2023-12-02 02:42 | Antoine A | Note Added: 0020681 | |
| 2023-12-02 07:40 | Christian Grothoff | Note Added: 0020682 | |
| 2023-12-02 07:52 | Antoine A | Note Added: 0020683 | |
| 2023-12-02 07:58 | Christian Grothoff | Note Added: 0020684 | |
| 2023-12-02 08:09 | Antoine A | Note Added: 0020685 | |
| 2023-12-02 10:01 | Christian Grothoff | Note Added: 0020686 | |
| 2023-12-02 14:57 | sebasjm | Note Added: 0020709 | |
| 2023-12-22 13:55 | Christian Grothoff | Target Version | 0.9.4 => post-1.0 |
| 2023-12-24 05:26 | Christian Grothoff | Status | acknowledged => assigned |
| 2024-07-01 09:54 | Antoine A | Status | assigned => feedback |
| 2024-07-01 09:54 | Antoine A | Note Added: 0022774 | |
| 2024-07-28 19:52 | Christian Grothoff | Severity | minor => tweak |
| 2025-05-08 20:42 | Christian Grothoff | Assigned To | Antoine A => |
| 2025-05-08 20:42 | Christian Grothoff | Status | feedback => acknowledged |
| 2025-05-08 20:42 | Christian Grothoff | Target Version | post-1.0 => |
| 2026-08-29 19:10 | Florian Dold | Target Version | => post-1.0 |