feat: add nextAppendPosition #24
Conversation
WalkthroughThe changes in this pull request involve modifications to the error handling logic in the Changes
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: 1
🧹 Outside diff range and nitpick comments (1)
src/OSSBaseClient.ts (1)
311-315: Approve changes and suggest type update for OSSClientErrorThe added code enhances error handling for append operations by including the 'nextAppendPosition' when encountering a 'PositionNotEqualToLength' error. This is a valuable addition that provides more context for error handling.
However, to improve type safety and avoid the use of
(err as any), consider updating theOSSClientErrortype to include thenextAppendPositionproperty.Suggestion: Update the
OSSClientErrorclass to include thenextAppendPositionproperty:export class OSSClientError extends Error { // ... existing properties ... nextAppendPosition?: string; constructor( // ... existing parameters ... nextAppendPosition?: string ) { // ... existing constructor logic ... this.nextAppendPosition = nextAppendPosition; } }Then update the error creation:
err = new OSSClientError(status, info?.Code ?? 'Unknown', message, requestId, hostId, result.headers['x-oss-next-append-position'] as string);This change will eliminate the need for type assertion and improve type safety throughout the codebase.
There was a problem hiding this comment.
Actionable comments posted: 3
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (1)
- test/OSSObject.test.ts (3 hunks)
🧰 Additional context used
🪛 GitHub Check: build (22, ubuntu-latest)
test/OSSObject.test.ts
[warning] 26-26:
describe.only not permitted
[warning] 172-172:
describe.only not permitted
🪛 GitHub Check: build (20, ubuntu-latest)
test/OSSObject.test.ts
[warning] 26-26:
describe.only not permitted
[warning] 172-172:
describe.only not permitted
🪛 GitHub Check: build (18, ubuntu-latest)
test/OSSObject.test.ts
[warning] 26-26:
describe.only not permitted
[warning] 172-172:
describe.only not permitted
🪛 GitHub Check: build (16, ubuntu-latest)
test/OSSObject.test.ts
[warning] 26-26:
describe.only not permitted
[warning] 172-172:
describe.only not permitted
🪛 Biome
test/OSSObject.test.ts
[error] 26-26: Don't focus the test.
The 'only' method is often used for debugging or during implementation. It should be removed before deploying to production.
Consider removing 'only' to ensure all tests are executed.
Unsafe fix: Remove focus from test.(lint/suspicious/noFocusedTests)
[error] 172-172: Don't focus the test.
The 'only' method is often used for debugging or during implementation. It should be removed before deploying to production.
Consider removing 'only' to ensure all tests are executed.
Unsafe fix: Remove focus from test.(lint/suspicious/noFocusedTests)
🔇 Additional comments (1)
test/OSSObject.test.ts (1)
Line range hint
1-1201: Overall assessment of the test fileThis test file for the OSSObject class appears to be comprehensive and well-structured. The main issues identified were:
- Two instances of
describe.onlywhich should be removed to ensure all tests are executed.- A type assertion to
anywhich could be replaced with a proper type definition update.Once these issues are addressed, the test suite will be more robust and maintainable. The extensive coverage of various scenarios and edge cases is commendable and should help ensure the reliability of the OSSObject class.
🧰 Tools
🪛 GitHub Check: build (22, ubuntu-latest)
[warning] 26-26:
describe.only not permitted🪛 GitHub Check: build (20, ubuntu-latest)
[warning] 26-26:
describe.only not permitted🪛 GitHub Check: build (18, ubuntu-latest)
[warning] 26-26:
describe.only not permitted🪛 GitHub Check: build (16, ubuntu-latest)
[warning] 26-26:
describe.only not permitted🪛 Biome
[error] 26-26: Don't focus the test.
The 'only' method is often used for debugging or during implementation. It should be removed before deploying to production.
Consider removing 'only' to ensure all tests are executed.
Unsafe fix: Remove focus from test.(lint/suspicious/noFocusedTests)
gxkl
force-pushed
the
feat/next-append-position-2
branch
from
4d57cc4 to
7142916
Compare
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## master #24 +/- ##
==========================================
- Coverage 94.37% 88.26% -6.12%
==========================================
Files 17 17
Lines 1885 1891 +6
Branches 285 249 -36
==========================================
- Hits 1779 1669 -110
- Misses 106 222 +116 ☔ View full report in Codecov by Sentry. |
[skip ci] ## [2.3.0](v2.2.0...v2.3.0) (2024-10-21) ### Features * add nextAppendPosition ([#24](#24)) ([f4bc68d](f4bc68d))
Summary by CodeRabbit
New Features
Bug Fixes
Tests