(API) Fix panic on hash failure #1259
Conversation
WalkthroughWalkthroughThe updates involve adjusting URLs for assets, specifically modifying Changes
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (invoked as PR comments)
Additionally, you can add CodeRabbit Configration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Review Status
Actionable comments generated: 0
Configuration used: CodeRabbit UI
Files selected for processing (4)
- api/adapter/controller.go (4 hunks)
- api/aggregator/controller.go (4 hunks)
- cli/src/adapter.ts (1 hunks)
- cli/src/aggregator.ts (1 hunks)
Additional comments: 4
cli/src/adapter.ts (1)
- 122-124: The modification to log the entire
response.dataobject instead of just themessageproperty when an error occurs in thehashHandlerfunction is a good improvement for debugging purposes. However, please ensure that logging this information does not inadvertently expose any sensitive data.api/adapter/controller.go (3)
- 62-62: Replacing panic with a specific error response in the
insertfunction whencomputeAdapterHashfails is a significant improvement. This change enhances the robustness of the application by preventing crashes and providing more informative feedback to the client.- 109-109: Replacing panic with a specific error response in the
hashfunction whencomputeAdapterHashfails is another excellent improvement. It enhances error handling by preventing application crashes and providing clearer feedback to the client.- 156-156: Updating the
computeAdapterHashfunction to return an error instead of panicking is a commendable change. It forms the basis for the improvements in error handling observed in theinsertandhashfunctions, making the application more resilient.
There was a problem hiding this comment.
Review Status
Actionable comments generated: 0
Configuration used: CodeRabbit UI
Files ignored due to path filters (1)
cli/package.jsonis excluded by:!**/*.json
Files selected for processing (1)
- cli/test/mockData.ts (2 hunks)
Additional comments: 2
cli/test/mockData.ts (2)
- 86-102: The updates to
adapterSourceandaggregatorSourceURLs to include the/baobab/path are consistent across theDATAFEED_BULK_0object. This change aligns with the PR's objective to adjust URLs for different assets. Ensure that the new URLs are correct and accessible.- 120-136: Similar to
DATAFEED_BULK_0, theDATAFEED_BULK_1object's updates toadapterSourceandaggregatorSourceURLs to include the/baobab/path are consistent and align with the PR's objectives. Additionally, the changes towalletAddressandwalletPrivateKeyvalues are noted. Verify that these credentials are mock or test values and not actual private keys.
nick-bisonai
changed the title
There was a problem hiding this comment.
Review Status
Actionable comments generated: 0
Configuration used: CodeRabbit UI
Files selected for processing (1)
- api/utils/utils.go (1 hunks)
Additional comments: 1
api/utils/utils.go (1)
- 126-127: The adjustment to return an empty slice of type
Twhen the result set is empty and there's no error is a good practice for robust error handling. This change ensures that the absence of data is not treated as an error condition, which is particularly useful for queries that might legitimately return no rows under normal circumstances.Please ensure that all parts of the application that call
QueryRowsare updated to correctly handle an empty slice as a valid response, not indicative of an error.
api/adapter/controller.go
Outdated
| @@ -152,7 +153,7 @@ func computeAdapterHash(data *AdapterInsertModel, verify bool) error { | |||
|
|
|||
| out, err := json.Marshal(input) | |||
| if err != nil { | |||
| panic(err) | |||
| return err | |||
There was a problem hiding this comment.
Minor. Rather than repeating the same error message everywhere where computAdapterHash is used, we could return it here.
| return err | |
| return fmt.Sprintf(failed to compute adapter hash: %w", err.Error()) |
And then just call as
err = computeAdapterHash(&payload, verify)
if err != nil {
return c.Status(fiber.StatusInternalServerError).SendString(err.Error())
}
There was a problem hiding this comment.
Review Status
Actionable comments generated: 0
Configuration used: CodeRabbit UI
Files selected for processing (4)
- api/adapter/controller.go (4 hunks)
- api/aggregator/controller.go (4 hunks)
- cli/src/adapter.ts (1 hunks)
- cli/src/aggregator.ts (1 hunks)
Files skipped from review as they are similar to previous changes (4)
- api/adapter/controller.go
- api/aggregator/controller.go
- cli/src/adapter.ts
- cli/src/aggregator.ts
Description
fix panic failure on hash error for adapter and aggregator
AS-IS
TO-BE
Type of change
Please delete options that are not relevant.
Checklist before requesting a review
Deployment
Summary by CodeRabbit
/baobab/and modified wallet information for different assets.