Fix OpenBanking mapping crash when a scalar field is returned as an array - #100
Conversation
…rray JsonDoc.GetValue<T> could throw an uncaught InvalidCastException while mapping an Open Banking BankPaymentDetail response. When a field such as created_on came back as a JSON array it was stored as a List<string>; Convert.ChangeType then failed and the catch block cast the list straight to the target type, throwing a second time. That surfaced to callers as a 500 during ReportingService.BankPaymentDetail(...).Execute(). The catch now returns the value when it is already the requested type, otherwise collapses an array to its first convertible element, and falls back to default(T) instead of re-throwing. This covers every field mapped through GetValue<T>, not just created_on. Adds GpEcomOpenBankingMappingTest covering the arrayed and scalar cases.
|
One follow-up worth flagging while reviewing. This PR stops the crash and recovers the value on a best-effort basis. There is a smaller, separate quirk behind it: Newtonsoft parses a scalar ISO date straight into a If a reviewer wants exact dates even on a malformed array, there are two options I deliberately kept out of this change to keep the blast radius small:
Happy to fold either in if you prefer, otherwise I would suggest tracking option 1 separately. This PR is scoped to the "do not 500" fix. |
Fixes #99. Also resolves #97.
Problem
ReportingService.BankPaymentDetail(...).Execute()can return a 500 that never reaches the caller'sResults. The crash is inJsonDoc.GetValue<T>(Utils/JsonUtils.cs), not in the mapping.When the gateway returns a field as a JSON array where a scalar is expected,
ParseObjectstores it as aList<string>.GetValue<T>then hitsConvert.ChangeType(List<string>, DateTime), which throwsInvalidCastException. The existing catch runsreturn (T)_dict[name], casting the list straight to the target type, so it throws a second time and that one is uncaught.Reported against
created_on, but it is not specific to that field. Every field inMapTransactionSummaryreads throughGetValue<T>and fails the same way if it arrives as an array.Stack trace from the field report:
JsonDoc.GetValue[T]OpenBankingMapping.MapTransactionSummaryOpenBankingMapping.MapReportResponse[T]OpenBankingProvider.ProcessReport[T]ReportBuilder.ExecuteChange
One method,
JsonDoc.GetValue<T>. The catch now:T(the original intent, e.g. a nestedJsonDoc),default(T)otherwise, instead of re-throwing.No public API change. The path only runs inside a catch that previously always threw, so the worst case turns a crash into a value or a
default(T).Tests
GpEcomOpenBankingMappingTest, two cases:created_onas an array. Throws on current master, passes here.created_onas a plain string. Guards the normal path, which stays exact.Verified red then green: reverting the
JsonUtils.cschange makes the array test fail with the exact reported exception,InvalidCastException: Unable to cast object of type 'System.Collections.Generic.List1[System.String]' to type 'System.DateTime'.Note for triage
Re-running the report later returns the transaction's current state, which is scalar, so it looks unreproducible. The payload that failed was the one from the original callback, not a fresh query. To confirm the live trigger, pull the raw stored callback JSON for the transaction rather than re-querying it.