-
Notifications
You must be signed in to change notification settings - Fork 401
feat/dataSync #494
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: master
Are you sure you want to change the base?
feat/dataSync #494
Changes from all commits
dad9aca
1679c35
a8d0d73
cd73f80
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,111 @@ | ||
| # yaml-language-server: $schema=https://coderabbit.ai/integrations/schema.v2.json | ||
|
|
||
| language: en-US | ||
|
|
||
| reviews: | ||
| # Review draft PRs so we get early feedback | ||
| drafts: true | ||
|
|
||
| # Enable high-level summary of changes | ||
| high_level_summary: true | ||
|
|
||
| # Add a poem... just kidding, disable it | ||
| poem: false | ||
|
|
||
| # Collapse walkthrough to keep PR comments clean | ||
| collapse_walkthrough: true | ||
|
|
||
| # Auto-review on every push | ||
| auto_review: | ||
| enabled: true | ||
| drafts: true | ||
|
|
||
| # Path-based review instructions | ||
| path_instructions: | ||
| - path: "src/core/**" | ||
| instructions: | | ||
| This is the core SDK module. Pay close attention to: | ||
| - Backward compatibility of public API changes | ||
| - Proper TypeScript typing (strict mode is enabled) | ||
| - No platform-specific code (Node.js, Web, React Native specifics belong in their respective platform directories) | ||
| - Thread safety considerations for shared state | ||
| - path: "src/core/endpoints/**" | ||
| instructions: | | ||
| These are REST API endpoint implementations. Review for: | ||
| - Correct request/response type definitions | ||
| - Proper error handling and status code mapping | ||
| - Consistent parameter validation | ||
| - Adherence to PubNub REST API contracts | ||
| - path: "src/core/types/**" | ||
| instructions: | | ||
| TypeScript type definitions. Ensure: | ||
| - Types are precise and not overly permissive (avoid `any`) | ||
| - Exported types maintain backward compatibility | ||
| - Proper use of generics and utility types | ||
| - path: "src/event-engine/**" | ||
| instructions: | | ||
| State-machine-based subscription management. Review for: | ||
| - Correct state transitions and edge cases | ||
| - No leaked subscriptions or event listeners | ||
| - Proper cleanup on state exit | ||
| - path: "src/entities/**" | ||
| instructions: | | ||
| High-level subscription API (Channel, ChannelGroup, etc.). Review for: | ||
| - Proper event handler lifecycle management | ||
| - Memory leak prevention (listener cleanup) | ||
| - path: "src/transport/**" | ||
| instructions: | | ||
| Platform-specific HTTP transport implementations. Review for: | ||
| - Proper timeout and cancellation handling | ||
| - Correct header management | ||
| - Error propagation consistency across platforms | ||
| - path: "src/node/**" | ||
| instructions: "Node.js platform implementation. Ensure no browser/DOM APIs are used." | ||
| - path: "src/web/**" | ||
| instructions: "Browser platform implementation. Ensure no Node.js-specific APIs (fs, crypto, etc.) are used." | ||
| - path: "src/react_native/**" | ||
| instructions: "React Native platform implementation. Verify compatibility with RN runtime." | ||
| - path: "test/**" | ||
| instructions: | | ||
| Test files. Review for: | ||
| - Adequate coverage of edge cases | ||
| - Proper use of mocks/stubs (Sinon + Nock) | ||
| - No flaky patterns (hardcoded timeouts, race conditions) | ||
| - Tests that actually assert meaningful behavior | ||
|
|
||
| # Tools configuration | ||
| tools: | ||
| # Enable GitHub checks integration | ||
| github-checks: | ||
| enabled: true | ||
| timeout_ms: 120000 | ||
|
|
||
| chat: | ||
| auto_reply: true | ||
|
|
||
| # Path filters - ignore generated/vendored/build output | ||
| path_filters: | ||
| # Build outputs | ||
| - "!dist/**" | ||
| - "!lib/**" | ||
| - "!upload/**" | ||
|
|
||
| # Dependencies | ||
| - "!node_modules/**" | ||
|
|
||
| # Unrelated working directories (not part of the SDK source) | ||
| - "!dataSync/**" | ||
| - "!new-dataSync/**" | ||
| - "!demo-app/**" | ||
| - "!loan_quote/**" | ||
| - "!docs/**" | ||
|
|
||
| # Lock files - too noisy, low review value | ||
| - "!package-lock.json" | ||
|
|
||
| # Config/metadata that rarely needs review | ||
| - "!.pubnub.yml" | ||
| - "!.vscode/**" | ||
|
|
||
| # Generated type bundles | ||
| - "!lib/types/**" | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -19,6 +19,10 @@ export enum TransportMethod { | |
| * Request will be sent using `PATCH` method. | ||
| */ | ||
| PATCH = 'PATCH', | ||
| /** | ||
| * Request will be sent using `PUT` method. | ||
| */ | ||
| PUT = 'PUT', | ||
|
Comment on lines
+22
to
+25
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🧩 Analysis chain🏁 Script executed: #!/bin/bash
# Description: Check signature method in middleware.ts for PUT handling
rg -n -A 15 'signature\(req: TransportRequest\)' src/transport/Repository: pubnub/javascript Length of output: 1212 Signature calculation must include PUT request bodies to prevent authentication failures. The Update the condition to include if (method === TransportMethod.POST || method === TransportMethod.PATCH || method === TransportMethod.PUT) {🤖 Prompt for AI Agents |
||
| /** | ||
| * Request will be sent using `DELETE` method. | ||
| */ | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -143,6 +143,29 @@ export class PubNubAPIError extends Error { | |
| ) { | ||
| errorData = errorResponse; | ||
| status = errorResponse.status; | ||
| } else if ( | ||
| 'errors' in errorResponse && | ||
| Array.isArray(errorResponse.errors) && | ||
| errorResponse.errors.length > 0 | ||
| ) { | ||
| // Handle DataSync-style structured error responses: | ||
| // { errors: [{ errorCode: "SYN-0008", message: "...", path: "/id" }] } | ||
| errorData = errorResponse; | ||
|
|
||
| const errors = errorResponse.errors as Array<{ | ||
| errorCode?: string; | ||
| message?: string; | ||
| path?: string; | ||
| }>; | ||
|
|
||
| message = errors | ||
| .map((e) => { | ||
| const parts: string[] = []; | ||
| if (e.errorCode) parts.push(e.errorCode); | ||
| if (e.message) parts.push(e.message); | ||
| return parts.join(': '); | ||
| }) | ||
| .join('; '); | ||
| } else errorData = errorResponse; | ||
|
Comment on lines
+146
to
169
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Handle edge case where error items lack both If all items in the Consider filtering out empty results: Proposed fix message = errors
.map((e) => {
const parts: string[] = [];
if (e.errorCode) parts.push(e.errorCode);
if (e.message) parts.push(e.message);
return parts.join(': ');
})
+ .filter((s) => s.length > 0)
.join('; ');
+
+ if (!message) message = 'Unknown error';🤖 Prompt for AI Agents |
||
|
|
||
| if ('error' in errorResponse && errorResponse.error instanceof Error) errorData = errorResponse.error; | ||
|
|
@@ -229,6 +252,35 @@ export class PubNubAPIError extends Error { | |
| }; | ||
| } | ||
|
|
||
| /** | ||
| * Format a user-facing error message for this API error. | ||
| * | ||
| * When the error contains structured details extracted from the service response | ||
| * (e.g., DataSync `errors` array), those details are included in the message. | ||
| * Otherwise, falls back to a generic description. | ||
| * | ||
| * @param operation - Request operation during which error happened. | ||
| * | ||
| * @returns Formatted error message string. | ||
| */ | ||
| public toFormattedMessage(operation: RequestOperation): string { | ||
| const fallback = 'REST API request processing error, check status for details'; | ||
|
|
||
| // When errorData contains a structured `errors` array, `this.message` was already | ||
| // constructed from it in `createFromServiceResponse` — prefer it over the generic fallback. | ||
| if ( | ||
| this.errorData && | ||
| typeof this.errorData === 'object' && | ||
| !('name' in this.errorData && 'message' in this.errorData && 'stack' in this.errorData) && | ||
| 'errors' in this.errorData && | ||
| Array.isArray((this.errorData as Record<string, unknown>).errors) | ||
| ) { | ||
| return `${operation}: ${this.message}`; | ||
| } | ||
|
|
||
| return fallback; | ||
| } | ||
|
|
||
| /** | ||
| * Convert API error object to PubNub client error object. | ||
| * | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Misplaced
draftsproperty at thereviewslevel.According to the CodeRabbit configuration schema, the
draftsproperty belongs underreviews.auto_review.drafts, not directly underreviews. The schema specifies"additionalProperties": falsefor thereviewsobject, so this property will be ignored.Since you already have
drafts: truecorrectly placed underauto_review(line 21), you can safely remove line 7.Proposed fix
📝 Committable suggestion
🤖 Prompt for AI Agents