System.FormatException when loading IonText from QLDBDriver
calvaradocl opened this issue · 10 comments
Hello,
We are trying to load an IonText that was generated by the AmazonQLDBClient class, particularly we are obtaining a block with the SDK using the GetBlockAsync Method, this returns the following IonText:
{blockAddress:{strandId:"CMVdau4riZ2CbrM3UBTQmS",sequenceNo:414},transactionId:"FCASohj2pvdCeLxG8nmEhO",blockTimestamp:"2020-10-21T12:37:52.086Z",blockHash:{{EBjWtIx2i4wy7PpxuZOPdeCEMsvlRjhWTok4QE/P+Yc=}},entriesHash:{{RSqMS6marQ65+7/rmzYBYScQl9pKSpTO6InYo9T/SPQ=}},previousBlockHash:{{H/jE6QW89EEfwj3ZtAOb6GXIibb6njfydqQZYK/UZVE=}},entriesHashList:[{{5v++bAsKGQfXLMdrCEiG9GtOk36+XNXRJnovMTlnjFM=}},{{8HnIw1h0r2/sa48v2SMgQHtZAJCHoBKE5LN1RkFgmAQ=}},{{kL+8LaL5MDXWwjfJnvqy93EuaKwddb0IWA/+pi2q9yA=}},{{lnOAuk7T2KsgCihidoIZz2mqAtKKZ33LuwR5Nt2z58I=}}],transactionInfo:{statements:[{statement:"UPDATE Listing SET UpdatedOn = ?, Status = ? WHERE Id = ?",startTime:2020-10-21T12:37:51.811Z,statementDigest:{{sdY39AuS4+N+j+tjmgcBq5Sl+Vg4vfqRwpwCGp8AKlU=}}}],documents:{'1e8aN98mSyP8JwSZ05VeQr':{tableName:"Listing",tableId:"DuK5zEwrpg98m4i5bYj3cQ",statements:[0,0]}}},revisions:[{blockAddress:{strandId:"CMVdau4riZ2CbrM3UBTQmS",sequenceNo:414},hash:{{8HnIw1h0r2/sa48v2SMgQHtZAJCHoBKE5LN1RkFgmAQ=}},data:{Id:"25694a0e-7a9b-4eab-90de-652670b240ae",CreatedOn:"21-10-2020 12:37:45",UpdatedOn:'21-10-2020 12:37:46',Status:Sold},metadata:{id:"1e8aN98mSyP8JwSZ05VeQr",version:6,txTime:2020-10-21T12:37:52.085Z,txId:"FCASohj2pvdCeLxG8nmEhO"}}]}
Then, we use the following line to parse this IonText with this project:
GetBlockResponse getBlockResponse = await qLDBClient.GetBlockAsync(getBlockRequest, cts);
Console.WriteLine(getBlockResponse.Block.IonText);
var ok = IonLoader.Default.Load(getBlockResponse.Block.IonText);
But when loading the IonText we obtain the following exception related to the TimeStamp value of the BlockText.
System.FormatException: 2020-10-21T13:18:46.911Z
at Amazon.IonDotnet.Timestamp.Parse(String s)
at Amazon.IonDotnet.Internals.Text.SystemTextReader.LoadScalarValue()
at Amazon.IonDotnet.Internals.Text.SystemTextReader.LoadOnce()
at Amazon.IonDotnet.Internals.Text.SystemTextReader.PrepareValue()
at Amazon.IonDotnet.Internals.Text.SystemTextReader.TimestampValue()
at Amazon.IonDotnet.Internals.PrivateIonWriterBase.WriteValueRecursively(IonType type, IIonReader reader)
at Amazon.IonDotnet.Internals.PrivateIonWriterBase.WriteContainerRecursively(IonType type, IIonReader reader)
at Amazon.IonDotnet.Internals.PrivateIonWriterBase.WriteValueRecursively(IonType type, IIonReader reader)
at Amazon.IonDotnet.Internals.PrivateIonWriterBase.WriteValues(IIonReader reader)
at Amazon.IonDotnet.Builders.IonLoader.WriteDatagram(IIonReader reader)
at Amazon.IonDotnet.Builders.IonLoader.Load(String ionText)
We reviewed the code and found out that it tries to parse a decimal value, but C# doesn't seem to understand decimals that don't start with 0, we downloaded the TImestamp.cs class and found out that when it tries to parse .086 the code throws an exception.
How can we get this running?
Hi, thanks for the bug report. Here's a pretty-printed version of the provided data for easier reading:
{
blockAddress:{
strandId:"CMVdau4riZ2CbrM3UBTQmS",
sequenceNo:414
},
transactionId:"FCASohj2pvdCeLxG8nmEhO",
blockTimestamp:"2020-10-21T12:37:52.086Z",
blockHash:{{ EBjWtIx2i4wy7PpxuZOPdeCEMsvlRjhWTok4QE/P+Yc= }},
entriesHash:{{ RSqMS6marQ65+7/rmzYBYScQl9pKSpTO6InYo9T/SPQ= }},
previousBlockHash:{{ H/jE6QW89EEfwj3ZtAOb6GXIibb6njfydqQZYK/UZVE= }},
entriesHashList:[
{{ 5v++bAsKGQfXLMdrCEiG9GtOk36+XNXRJnovMTlnjFM= }},
{{ 8HnIw1h0r2/sa48v2SMgQHtZAJCHoBKE5LN1RkFgmAQ= }},
{{ kL+8LaL5MDXWwjfJnvqy93EuaKwddb0IWA/+pi2q9yA= }},
{{ lnOAuk7T2KsgCihidoIZz2mqAtKKZ33LuwR5Nt2z58I= }}
],
transactionInfo:{
statements:[
{
statement:"UPDATE Listing SET UpdatedOn = ?, Status = ? WHERE Id = ?",
startTime:2020-10-21T12:37:51.811Z,
statementDigest:{{ sdY39AuS4+N+j+tjmgcBq5Sl+Vg4vfqRwpwCGp8AKlU= }}
}
],
documents:{
'1e8aN98mSyP8JwSZ05VeQr':{
tableName:"Listing",
tableId:"DuK5zEwrpg98m4i5bYj3cQ",
statements:[
0,
0
]
}
}
},
revisions:[
{
blockAddress:{
strandId:"CMVdau4riZ2CbrM3UBTQmS",
sequenceNo:414
},
hash:{{ 8HnIw1h0r2/sa48v2SMgQHtZAJCHoBKE5LN1RkFgmAQ= }},
data:{
Id:"25694a0e-7a9b-4eab-90de-652670b240ae",
CreatedOn:"21-10-2020 12:37:45",
UpdatedOn:'21-10-2020 12:37:46',
Status:Sold
},
metadata:{
id:"1e8aN98mSyP8JwSZ05VeQr",
version:6,
txTime:2020-10-21T12:37:52.085Z,
txId:"FCASohj2pvdCeLxG8nmEhO"
}
}
]
}We reviewed the code and found out that it tries to parse a decimal value, but C# doesn't seem to understand decimals that don't start with 0, we downloaded the TImestamp.cs class and found out that when it tries to parse .086 the code throws an exception.
Could you provide a link to the code you're referring to? I'm able to run the following test code without getting exceptions:
[DataRow("2020-10-21T12:37:52.086Z"), DataRow("2020-10-21T12:37:52.186Z"), DataRow("2020-10-21T12:37:00.086Z"), DataRow("2020-10-21T13:18:46.911Z"), DataRow("2020-10-21T13:18:00.911Z")]
[TestMethod]
public void Date_FractionalSecondsStartWithZero(string dateString)
{
var date = Timestamp.Parse(dateString);
Console.WriteLine(date);
}As an aside: I notice that the blockTimestamp field is formatted like a timestamp (2020-10-21T12:37:52.086Z) but is wrapped in double quotes ("2020-10-21T12:37:52.086Z"), making it an Ion string rather than an Ion timestamp. That shouldn't cause parse errors, but it stands out to me. Is that field created/managed by the QLDB client?
I was also able to parse a string containing the complete example data without an exception.
var ionValue = IonLoader.Default.Load(data);
Console.WriteLine(ionValue.ToString());outputs
Amazon.IonDotnet.Tree.Impl.IonDatagram
(A trivial example, but it seems to parse without issue.)
Hi @zslayton, sorry about the double quotes, that was a hand-made modification from us when testing, the Ion text returned by the QLDB client doesn't have those. We also wanted to check if the missing double quotes were part of the problem, but even if we added them the code still failed.
We are working with the following Nuget Packages for this issue:
- AWSSDK.QLDB v 3.5.0.29
- Amazon.QLDB.driver v 1.0.1
- Amazon.IonDotNet 1.0.0
We got the same exception when running the Timestamp.Parse(string s) method, when the following method is called on line 423:
/// <summary>
/// Parse a substring to decimal.
/// </summary>
private static bool DecimalTryParseSubString(string s, int offset, int length, out decimal output)
{
if (offset + length > s.Length)
{
output = 0;
return false;
}
return decimal.TryParse(s.Substring(offset, length), out output);
}
Once the decimal.TryParse(s.Substring(offset, length), out output); the method is called, it obtains a substring with the .086 value, and when running the decimal.TryParse method, it fails (returning false).
I think the issue can be related to how the string is being parsed considering the Culture of the runtime environment (ours is es-CL), if we set it up to be en-US it works.
Check the folllowing example:
When running the parse method on en-US the code doesn't fail.
CultureInfo.CurrentCulture = new CultureInfo("en-US", false); // This should be your cultureinfo
Console.WriteLine("CurrentCulture is {0}.", CultureInfo.CurrentCulture.Name);
decimal.Parse(".086"); // Works
When running on es-CL culture, the code fails.
CultureInfo.CurrentCulture = new CultureInfo("es-CL", false); // We have this by default
Console.WriteLine("CurrentCulture is {0}.", CultureInfo.CurrentCulture.Name);
decimal.Parse(".086"); // FAILS
If the TryParse method uses the CultureInfo.InvariantCulture provider as a parameter, it can parse that type of string on different environments.
sorry about the double quotes, that was a hand-made modification from us when testing, the Ion text returned by the QLDB client doesn't have those. We also wanted to check if the missing double quotes were part of the problem, but even if we added them the code still failed.
Got it, thanks for explaining. 👍
I think the issue can be related to how the string is being parsed considering the Culture of the runtime environment (ours is es-CL), if we set it up to be en-US it works.
Wow, great find! You're right, now I can reproduce the issue.
I think I can replace the call to TryParse(String, Decimal) with TryParse(String, NumberStyles, IFormatProvider, Decimal). I'll put together a patch with this change.
Can you give me a sense of the impact of this issue? Are you blocked, or can you use binary Ion as a workaround until a fix is available?
Currently, we are working on implementing QLDB for one of our projects, this specific issue is related to our verification mechanism with QLDB and we need to parse the object to access its internal data as part of the process, so for now without the patch live, we won't be able to have that feature working.
We use the NuGet Libraries (on an ASP Core project). I don't know how much time takes for you to deploy a new version over that package manager, it would be great if you could give us a hint of how much time or when are you planning to release an updated version.
so for now without the patch live, we won't be able to have that feature working.
I expected a production system to be using binary Ion (not text) for performance reasons, so I thought this might not be a show-stopper.
I have a PR out with a fix and associated unit tests. I can't commit to a particular date to publish a new version in NuGet, but it shouldn't take too long.
Hi Zack,
Im working with carlos on this implementation of QLDB using .Net, and I dint get your advice of using binary ion (I dont see it on the libraries for QLDB),
What is our problem? we try to get the values that return the qldb Client for a block (specificly the class GetBlockResponse)
the data of the block is on the field called Block (a ValueHolder type), and the only way to get the info from the block is using that Iontext inside of the field "Block".
So, Getting the IonText of the block, I can parse it to a IionValue:
IIonValue finalBlock = IonLoader.Default.Load(getBlockResponse.Block.IonText);
at that point we get the exception described by Carlos, I use the IionValue because with this we can call a specific field of the block
like. For example :
Console.WriteLine(finalBlock.GetField("staments").GetField("stament").StringValue);
So, my question is if there is another way to work with the GetBlockResponse.Block.IonText property (Like the binary ion that you said previously)
I appreciate your help with this issue!
So, my question is if there is another way to work with the GetBlockResponse.Block.IonText property (Like the binary ion that you said previously)
Sorry, I'm not familiar enough with the QLDB client libraries to say for sure. It may be that they always use text Ion for that data because it's human-readable (albeit slower).
The PR has been merged and will be included in the next release.
Hi @zslayton, ok, thanks for the feedback on this update.
The fix for this (#129) is now available in v1.1.0 on NuGet.