chore: added test to verify error #2746
Conversation
MichaelUnkey
requested review from
perkinsjr,
chronark,
mcstepp and
ogzhanolguncu
as code owners
|
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
📝 WalkthroughWalkthroughThe changes in this pull request introduce a new test case to the existing test suite for the Changes
Possibly related PRs
Suggested reviewers
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 (
|
|
Thank you for following the naming conventions for pull request titles! 🙏 |
|
From Andreas Thomas ‣ ``` |
![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 (1)
packages/api/src/client.test.ts (1)
15-28: Consider enhancing the test coverage and readability.A few suggestions to improve the test:
- Use a more descriptive variable name than 'res'
- Consider using
test.fails()to explicitly indicate that we're testing an error scenario- Add more test cases for different error scenarios
Here's a suggested improvement:
- test("errors are correctly passed through to the caller", async () => { - const unkey = new Unkey({ rootKey: "wrong key" }); - const res = await unkey.keys.create({ + test.fails("should handle unauthorized error correctly", async () => { + const unkey = new Unkey({ rootKey: "wrong key" }); + const response = await unkey.keys.create({ apiId: "", }); - expect(res.error).toBeDefined(); - expect(res.error!.code).toEqual("UNAUTHORIZED"); - expect(res.error!.docs).toEqual( + expect(response.error).toBeDefined(); + expect(response.error!.code).toEqual("UNAUTHORIZED"); + expect(response.error!.docs).toEqual( "https://unkey.dev/docs/api-reference/errors/code/UNAUTHORIZED", ); - expect(res.error!.message).toEqual("key not found"); - expect(res.error!.requestId).toBeDefined(); + expect(response.error!.message).toEqual("key not found"); + expect(response.error!.requestId).toBeDefined(); });
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
packages/api/src/client.test.ts(1 hunks)
🔇 Additional comments (2)
packages/api/src/client.test.ts (2)
15-28: LGTM! The test case effectively verifies error handling.
The test successfully validates the error response structure and content when using an invalid root key.
17-19: Verify the error handling in the client implementation.
Let's verify that the client properly handles various error scenarios.
✅ Verification successful
Error handling is properly implemented in the client
The client implementation has comprehensive error handling:
- Network errors are handled with retries (configurable attempts and backoff)
- API errors are properly parsed and returned with code, message, docs, and requestId
- Empty/undefined parameters are handled gracefully
- The test file specifically verifies error scenarios with invalid credentials
The test case in question correctly verifies that API errors are properly propagated to the caller, including all required error fields.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Check the error handling implementation in the client code
# Look for error handling patterns in the client implementation
ast-grep --pattern 'error: {
$$$
}'
# Check for other test files that might need similar error handling tests
fd -e test.ts -e spec.ts --exec grep -l "new Unkey"
Length of output: 362
Script:
#!/bin/bash
# Let's check the error handling patterns in the client implementation and test files
# Check error handling in client implementation
rg -A 5 "throw|catch|error:" packages/api/src/client.ts
# Check error handling in test files
rg -A 5 "expect.*throw|expect.*reject" packages/api/src/client.test.ts
# Look at the actual implementation of key creation
ast-grep --pattern 'keys = {
$$$
create($$$) {
$$$
}
$$$
}'
Length of output: 986
Script:
#!/bin/bash
# Let's examine the key creation implementation and its error handling more specifically
# Look for the create key implementation
rg -A 10 "create\(.*\).*{" packages/api/src/client.ts
# Check for any validation or error handling specific to apiId
rg -A 5 "apiId" packages/api/src/client.ts
# Look for any error-related types or interfaces
rg -A 5 "interface.*Error|type.*Error" packages/api/src/client.ts
Length of output: 438
Script:
#!/bin/bash
# Let's try a different approach to examine the implementation
# Look for the create method with fixed pattern
rg -A 15 'create.*\{' packages/api/src/client.ts
# Check the error types definition
cat packages/api/src/errors.ts
# Look for any request handling patterns
rg -A 10 'async request' packages/api/src/client.ts
Length of output: 337
Script:
#!/bin/bash
# Let's examine the entire client implementation since previous commands didn't capture enough context
# View the complete client implementation
cat packages/api/src/client.ts
# Check test file for error scenarios
cat packages/api/src/client.test.ts
Length of output: 16513
|
From Andreas Thomas ‣ ok I found it, I’ll address it |
What does this PR do?
Fixes # (issue)
Eng-1599
If there is not an issue for this, please create one first. This is used to tracking purposes and also helps use understand why this PR exists
Type of change
How should this be tested?
Checklist
Required
pnpm buildpnpm fmtconsole.logsgit pull origin mainAppreciated
Summary by CodeRabbit
createmethod of thekeysAPI.