fix(openapi): bulk response definition #854
Conversation
WalkthroughThis update removes the "details" field from error response examples in the API documentation and updates the bulk response model. The changes include modifying required field flags in the documentation, converting certain fields in the Changes
Sequence Diagram(s)sequenceDiagram
participant Client
participant Generator
participant BulkResponse
Client->>Generator: Send request causing error (e.g., bad cursor)
Generator->>BulkResponse: Retrieve ErrorMessage & ErrorCode pointers
BulkResponse-->>Generator: Return pointer values (nil if absent)
Generator->>Client: Respond with HTTP 400 and simplified error response (without "details")
Assessment against linked issues
Possibly related PRs
Poem
✨ Finishing Touches
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 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 (
|
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #854 +/- ##
==========================================
+ Coverage 82.17% 82.46% +0.28%
==========================================
Files 139 140 +1
Lines 7536 7595 +59
==========================================
+ Hits 6193 6263 +70
+ Misses 1027 1017 -10
+ Partials 316 315 -1 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
![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
🧹 Nitpick comments (1)
docs/api/README.md (1)
3869-3869: Duplicate Heading Warning:
Static analysis indicates that there are multiple headings with the same content (MD024). Although this can sometimes be intentional in API documentation, it could also potentially reduce clarity. Consider reviewing these duplicated headings to determine if they can be made unique or supplemented with additional context. If the duplications are intentional, it might be helpful to add a comment to explain the rationale.🧰 Tools
🪛 markdownlint-cli2 (0.17.2)
3869-3869: Multiple headings with the same content
null(MD024, no-duplicate-heading)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (3)
openapi.yamlis excluded by!**/*.yamlopenapi/v2.yamlis excluded by!**/*.yamlpkg/client/.speakeasy/gen.lockis excluded by!**/*.lock,!**/*.lock
📒 Files selected for processing (4)
docs/api/README.md(2 hunks)pkg/client/docs/models/components/v2bulkresponse.md(1 hunks)pkg/client/models/components/v2bulkresponse.go(1 hunks)pkg/generate/generator.go(1 hunks)
🧰 Additional context used
🧬 Code Definitions (2)
pkg/generate/generator.go (1)
pkg/client/models/components/v2bulkresponse.go (1)
V2BulkResponse(5-9)
pkg/client/models/components/v2bulkresponse.go (2)
pkg/client/models/components/v2bulkelementresult.go (1)
V2BulkElementResult(157-165)pkg/client/models/components/v2errorsenum.go (1)
V2ErrorsEnum(10-10)
🪛 markdownlint-cli2 (0.17.2)
docs/api/README.md
3869-3869: Multiple headings with the same content
null
(MD024, no-duplicate-heading)
🔇 Additional comments (7)
pkg/client/docs/models/components/v2bulkresponse.md (1)
6-10: Documentation properly reflects API changesThe table updates accurately reflect the changes in the V2BulkResponse struct, where fields are now optional with omitempty tags. This maintains consistency between code and documentation.
pkg/client/models/components/v2bulkresponse.go (5)
6-8: Appropriate conversion of fields to optional with omitemptyThe conversion of Data, ErrorCode, and ErrorMessage fields to be optional (with omitempty tag) and the conversion of ErrorCode and ErrorMessage to pointer types is appropriate. This allows these fields to be omitted from JSON responses when they're not present.
11-16: Improved nil handling in GetData methodThe GetData method now correctly returns nil instead of an empty slice when the receiver is nil, which is consistent with the behavior of the other getter methods.
18-23: Consistent update to GetErrorCode methodThe GetErrorCode method has been properly updated to return a pointer type, maintaining consistency with the field type change in the struct.
25-30: Consistent update to GetErrorMessage methodThe GetErrorMessage method has been properly updated to return a pointer type, maintaining consistency with the field type change in the struct.
1-30: Removed GetDetails methodThe GetDetails method has been removed, which is consistent with the removal of the Details field from the struct and documentation. This simplifies the error response structure.
docs/api/README.md (1)
3766-3866: Bulk Response Schema Update – "details" Field Removal:
The change in the V2BulkResponse schema correctly removes the"details"field from the error response example. This aligns with the PR objective of simplifying and correcting the bulk response definition. Please verify that the client code or any downstream documentation that previously expected the"details"field is updated accordingly.
| *response.V2BulkResponse.ErrorMessage, | ||
| *response.V2BulkResponse.ErrorCode, |
There was a problem hiding this comment.
Check for nil pointers before dereferencing ErrorMessage and ErrorCode
The code now properly dereferences the ErrorMessage and ErrorCode pointers from the V2BulkResponse struct. However, there's no nil check before dereferencing, which could lead to a panic if these fields are nil in the response.
Consider adding nil checks before dereferencing these pointers:
if response.HTTPMeta.Response.StatusCode == http.StatusBadRequest {
+ var errorMsg string
+ if response.V2BulkResponse.ErrorMessage != nil {
+ errorMsg = *response.V2BulkResponse.ErrorMessage
+ } else {
+ errorMsg = "unknown error"
+ }
+
+ var errorCode string
+ if response.V2BulkResponse.ErrorCode != nil {
+ errorCode = string(*response.V2BulkResponse.ErrorCode)
+ } else {
+ errorCode = "UNKNOWN"
+ }
+
return nil, fmt.Errorf(
"unexpected error: %s [%s]",
- *response.V2BulkResponse.ErrorMessage,
- *response.V2BulkResponse.ErrorCode,
+ errorMsg,
+ errorCode,
)
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| *response.V2BulkResponse.ErrorMessage, | |
| *response.V2BulkResponse.ErrorCode, | |
| if response.HTTPMeta.Response.StatusCode == http.StatusBadRequest { | |
| var errorMsg string | |
| if response.V2BulkResponse.ErrorMessage != nil { | |
| errorMsg = *response.V2BulkResponse.ErrorMessage | |
| } else { | |
| errorMsg = "unknown error" | |
| } | |
| var errorCode string | |
| if response.V2BulkResponse.ErrorCode != nil { | |
| errorCode = string(*response.V2BulkResponse.ErrorCode) | |
| } else { | |
| errorCode = "UNKNOWN" | |
| } | |
| return nil, fmt.Errorf( | |
| "unexpected error: %s [%s]", | |
| errorMsg, | |
| errorCode, | |
| ) | |
| } |
Fixes LX-33