fix: Make data retrieval more resilient. Do not throw on ipfs data problems. #375
Conversation
Codecov ReportAttention:
❗ Your organization needs to install the Codecov GitHub app to enable full functionality. Additional details and impacted files@@ Coverage Diff @@
## develop #375 +/- ##
============================================
- Coverage 83.18% 32.98% -50.20%
============================================
Files 19 19
Lines 1231 1243 +12
Branches 282 286 +4
============================================
- Hits 1024 410 -614
- Misses 110 802 +692
+ Partials 97 31 -66 ☔ View full report in Codecov by Sentry. |
…cessible withing getDataFromExternalSources.
richtera
force-pushed
the
fix/cleanup-ipfs-retrieval
branch
from
d8e80e5 to
2a18f2b
Compare
|
Rebased. |
| if (numberOfBytes > (uintLength as number) / 8) { | ||
| throw new Error( | ||
| `Can't convert hex value ${value} to ${type}. Too many bytes. ${numberOfBytes} > 16`, | ||
| console.error( |
There was a problem hiding this comment.
I think this is incorrect and this part might need to be refactored. If there is more bytes than number of bits / 8, it should be fine as long as the left most bytes are 00.
If for instance for a uint8, there is more than 1 byte, that's ok as long as the left most byte is 00.
For instance, to decoding 0x00fe as uint8 would be ok, it will decode as 254 ✅
But if you try to decode 0x01fe as uint8, it should not allow you ❌
| @@ -782,7 +786,7 @@ export const valueContentEncodingMap = ( | |||
| }, | |||
| decode: (value: string) => { | |||
| if (typeof value !== 'string' || !isHex(value)) { | |||
| console.log(`Value: ${value} is not hex.`); | |||
| console.error(`Value: ${value} is not hex.`); | |||
There was a problem hiding this comment.
Nice idea to make it error in the console!
What kind of change does this PR introduce (bug fix, feature, docs update, ...)?
What is the current behaviour (you can also link to an open issue here)?
The current behavior is that when data is invalid or unreachable in ipfs. getDataFromExternalSource will throw an exception.
What is the new behaviour (if this is a feature change)?
These are now converted to a console.error, allowing the data to return as "null". This allows getData to be called with multiple keys
including one that maybe invalid and not just throw out of the section of code.
Other information:
I also made a similar kind of fix when the array length data (i.e. LSP12IssuedAssets[]) contains a uint256 instead of a uint128.