feat: add nextAppendPosition #22
Conversation
WalkthroughThe changes involve modifications to 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: Enhance error handling for 'PositionNotEqualToLength' errorThis addition improves error handling for append operations by including the 'nextAppendPosition' when available. This information can be crucial for clients to handle this specific error case effectively.
Consider the following improvements:
- Define a custom error type that includes the 'nextAppendPosition' property to avoid using 'any' type casting.
- Add a comment explaining the significance of this error case and how clients should use the 'nextAppendPosition' information.
Here's a suggested implementation:
// Define a custom error type interface OSSAppendPositionError extends OSSClientError { nextAppendPosition?: string; } // In the #createClientException method // ... // Handle PositionNotEqualToLength error if (info?.Code === 'PositionNotEqualToLength' && result.headers['x-oss-next-append-position']) { // This error occurs when the position specified in an append operation doesn't match the current object length. // The 'nextAppendPosition' indicates the correct position for the next append operation. (err as OSSAppendPositionError).nextAppendPosition = result.headers['x-oss-next-append-position']; } // ...This approach improves type safety and provides better context for the error handling.
| @@ -404,6 +404,7 @@ describe('test/OSSObject.test.ts', () => { | |||
| assert.equal(err.name, 'OSSClientError'); | |||
| assert.equal(err.code, 'PositionNotEqualToLength'); | |||
| assert.equal(err.status, 409); | |||
| assert.equal((err as any).nextAppendPosition, '3'); | |||
There was a problem hiding this comment.
🛠️ Refactor suggestion
Enhance type safety by updating OSSClientError to include nextAppendPosition property.
Currently, nextAppendPosition is accessed using a type assertion (err as any), which bypasses TypeScript's type checking and may lead to runtime errors. To improve type safety and code clarity, consider adding the nextAppendPosition property to the OSSClientError class definition.
You can update the OSSClientError class as follows:
// In src/error/OSSClientError.ts
export class OSSClientError extends Error {
code: string;
status: number;
// Existing properties...
// Add the nextAppendPosition property
nextAppendPosition?: string;
// Constructor and other methods...
}After updating the class, you can remove the type assertion in your test:
- assert.equal((err as any).nextAppendPosition, '3');
+ assert.equal(err.nextAppendPosition, '3');
Summary by CodeRabbit
New Features
Bug Fixes
Tests