fix(priceprovider-bybit): add special exception for blocked regions in bybit test #55
Conversation
WalkthroughThe changes in this pull request involve updates to the Changes
Possibly related issues
Poem
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 using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Outside diff range and nitpick comments (2)
feeder/priceprovider/sources/bybit.go (2)
32-33: LGTM: New constant added for improved error handling.The
ErrBybitBlockAccessconstant is well-defined and follows Go naming conventions. It will be useful for identifying specific error conditions related to geographical access restrictions.Consider adding a brief comment above the constant to explain its purpose, for example:
// ErrBybitBlockAccess is the error message returned when access is blocked due to geographical restrictions. const ErrBybitBlockAccess = "configured to block access from your country"
47-47: LGTM: Improved variable naming and error handling.The changes enhance the readability and error handling capabilities of the
BybitPriceUpdatefunction:
- Renaming
btorespBodyimproves code clarity.- The new error handling logic for blocked access provides more context when this specific error occurs.
Consider using a custom error type instead of wrapping the error with a string. This would allow for more structured error handling:
type BybitBlockedAccessError struct { Err error } func (e *BybitBlockedAccessError) Error() string { return fmt.Sprintf("%s: %v", ErrBybitBlockAccess, e.Err) } // In the BybitPriceUpdate function: if strings.Contains(string(respBody), ErrBybitBlockAccess) { return nil, &BybitBlockedAccessError{Err: err} }This approach would make it easier for callers to check for this specific error type if needed.
Also applies to: 55-59
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (2)
- feeder/priceprovider/sources/bybit.go (3 hunks)
- feeder/priceprovider/sources/bybit_test.go (1 hunks)
🧰 Additional context used
🔇 Additional comments (3)
feeder/priceprovider/sources/bybit_test.go (1)
16-19: 🛠️ Refactor suggestionImprove error handling and test coverage
The addition of error handling for
ErrBybitBlockAccessis a good improvement. However, there are a few points to consider:
The
ErrBybitBlockAccessconstant is not defined in this file. Consider adding a comment to explain its origin or import it explicitly for clarity.The current implementation might mask other potential errors. Consider logging the error or asserting that it's specifically
ErrBybitBlockAccessrather than just containing the error message.The test might benefit from additional cases to cover other potential error scenarios.
Consider refactoring the error handling as follows:
if err != nil { - require.ErrorContains(t, err, ErrBybitBlockAccess) - return + if errors.Is(err, ErrBybitBlockAccess) { + t.Log("Bybit access blocked, skipping further assertions") + return + } + t.Fatalf("Unexpected error: %v", err) }This change will:
- Use
errors.Isfor more precise error checking- Log a message explaining why further assertions are skipped
- Fail the test for unexpected errors, improving test coverage
To ensure
ErrBybitBlockAccessis properly defined, run:Would you like assistance in adding more test cases to cover other error scenarios?
feeder/priceprovider/sources/bybit.go (2)
5-5: LGTM: Import statements updated appropriately.The addition of the "fmt" and "strings" packages is consistent with the new error handling logic introduced in this change.
Also applies to: 9-9
Line range hint
1-85: Summary: Improved error handling for blocked regions in Bybit.The changes in this file effectively address the PR objective of adding a special exception for blocked regions in Bybit tests. Key improvements include:
- Addition of a new constant
ErrBybitBlockAccessfor clear error messaging.- Enhanced error handling in the
BybitPriceUpdatefunction to detect and report geographical access restrictions.- Improved variable naming for better code readability.
These changes will make it easier to identify and handle cases where Bybit blocks access based on geographical location, improving the robustness of the price provider implementation.
Summary by CodeRabbit
New Features
Bug Fixes