Client validation #139
Client validation #139
Conversation
WalkthroughThe changes in this pull request introduce a new file for validating API responses using Zod, alongside several updates to existing files to enhance validation functionality. Key modifications include the addition of new types and functions for request and response validation, improvements to the Changes
Possibly related PRs
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: 12
🧹 Outside diff range and nitpick comments (15)
src/utils.ts (1)
55-63: LGTM! Consider adding duplicate key handlingThe implementation is clean and type-safe. One minor suggestion would be to add explicit handling for duplicate keys, either by:
- Throwing an error on duplicates
- Documenting that later values override earlier ones
- Providing an option to customize the behavior
Example enhancement:
export function tupleIteratorToObject<T extends string | number | symbol, U>( iterator: Iterable<[T, U]>, + options: { onDuplicate?: 'error' | 'override' | 'skip' } = { onDuplicate: 'override' } ): Record<T, U> { const result = {} as Record<T, U>; for (const [key, value] of iterator) { + if (key in result && options.onDuplicate !== 'override') { + if (options.onDuplicate === 'error') { + throw new Error(`Duplicate key found: ${String(key)}`); + } + continue; // skip + } result[key] = value; } return result; }src/express/valibot.ts (1)
Line range hint
31-46: Consider updating the documentation example.The JSDoc example might need to be updated to demonstrate the new validation structure, particularly if there are new response validation capabilities available through the refactored system.
Would you like me to propose an updated example that showcases the new validation patterns?
examples/simple/withValidation.ts (3)
5-5: Consider making the API origin configurableConsider using an environment variable for the API origin to make the example more flexible for testing and different environments.
-const GITHUB_API_ORIGIN = "https://api.github.com"; +const GITHUB_API_ORIGIN = process.env.GITHUB_API_ORIGIN || "https://api.github.com";
7-14: Enhance API specification documentationWhile the GitHub API link is helpful, consider adding more detailed documentation about:
- Expected response format
- Required headers
- Possible error responses
24-56: Consider making the example more production-readyWhile this example effectively demonstrates the validation functionality, consider enhancing it with:
- Proper logging mechanism instead of console.log
- Retry mechanism for transient failures
- Timeout handling
- Rate limiting consideration for GitHub API
src/core/spec.ts (2)
41-43: LGTM! Consider documenting usage patterns.The
UnknownApiEndpointstype is well-structured and maintains consistency with existing types. It provides a flexible way to handle API endpoints where the exact structure isn't known at compile time.Consider documenting common usage patterns, such as:
- Runtime validation of external API responses
- Dynamic API client generation
- API mocking and testing scenarios
73-79: Consider enhancing type inference capabilities.The
UnknownApiSpectype is well-implemented, but could benefit from better type inference capabilities.Consider this enhancement to improve type inference:
-export type UnknownApiSpec = BaseApiSpec< - unknown, - unknown, - unknown, - unknown, - DefineApiResponses<DefineResponse<unknown, unknown>> ->; +export type UnknownApiSpec< + TParams = unknown, + TQuery = unknown, + TBody = unknown, + THeaders = unknown, + TResponse = unknown +> = BaseApiSpec< + TParams, + TQuery, + TBody, + THeaders, + DefineApiResponses<DefineResponse<TResponse, unknown>> +>;This change would allow for partial type inference when some types are known while keeping the flexibility for unknown types.
src/express/zod.test.ts (1)
306-306: Remove debugging console.log statement.This appears to be a debugging statement that should be removed before merging.
- console.log(res.body);src/core/validate.ts (1)
57-61: Address the TODO: Investigate why the body validator is not defined when the schema is incorrect.There's a TODO comment indicating that the
bodyvalidator is not defined when the schema is incorrect. This might lead to unexpected validation behavior or missed validation errors. Consider exploring this issue to ensure that thebodyvalidator functions correctly even when the schema has errors.Would you like assistance in resolving this issue or opening a GitHub issue to track this task?
src/zod/index.ts (1)
Line range hint
90-125: Refactor duplicated logic betweenreqandresfunctionsThe
reqandresfunctions share similar logic, such as callingpreCheckand constructingzodValidators. Consider extracting the common parts into a shared helper function to reduce duplication and improve maintainability.Also applies to: 126-156
src/express/index.ts (2)
104-104: Renamev2tovalidatorfor clarity and consistency.The variable
v2is not descriptive and may cause confusion. Since you're destructuringvalidatorfrom the returned object, consider using the same name to enhance readability and maintain consistency.Apply this diff to rename the variable:
- const { validator: v2 } = validator({ + const { validator } = validator({And update the return statement accordingly:
- return v2; + return validator;
112-112: Returnvalidatordirectly after renaming.After renaming
v2tovalidator, you can return it directly without any changes, simplifying the code and improving clarity.Apply this diff to update the return statement:
- return v2; + return validator;src/valibot/index.ts (3)
107-107: Rename 'zodValidators' to 'valibotValidators' for clarityIn both the
reqandresfunctions, the variablezodValidatorsis used to store validators generated using Valibot schemas, not Zod. Renaming this variable tovalibotValidatorsor simplyvalidatorswould improve code readability and avoid confusion.Apply this diff to rename the variable:
-const zodValidators: Record<string, any> = {}; +const valibotValidators: Record<string, any> = {};Remember to update all references to
zodValidatorswithin these functions accordingly.Also applies to: 145-145
97-99: Remove the unusedParamKeystype parameter fromValibotValidatorsThe
ParamKeystype parameter inValibotValidatorsis currently unused, as indicated by the// FIXMEcomment and the ESLint directive. This can lead to unnecessary complexity in the type definitions. Consider removingParamKeysfromValibotValidatorsto simplify the type signatures.Apply this diff to modify the
ValibotValidatorstype definition:-export type ValibotValidators< - AS extends ValibotApiSpec, - // FIXME - // eslint-disable-next-line @typescript-eslint/no-unused-vars - ParamKeys extends string, -> = Validators< +export type ValibotValidators< + AS extends ValibotApiSpec +> = Validators< ValibotValidator<AS["params"]>, ValibotValidator<AS["query"]>, ValibotValidator<AS["body"]>, ValibotValidator<AS["headers"]> >;Then, update the usages in the
reqandresfunctions:-? ValibotValidators<E[Path][M], ""> +? ValibotValidators<E[Path][M]>Also applies to: 134-136
Line range hint
94-130: Refactor to eliminate duplicate logic inreqandresfunctionsThe
reqandresfunctions share similar logic, including error handling withpreCheckand the construction of validators. Extracting the common code into a shared utility function could reduce duplication and enhance maintainability.Also applies to: 131-160
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
⛔ Files ignored due to path filters (1)
package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (13)
examples/simple/withValidation.ts(1 hunks)package.json(2 hunks)src/core/spec.ts(2 hunks)src/core/validate.ts(3 hunks)src/express/index.ts(1 hunks)src/express/valibot.test.ts(1 hunks)src/express/valibot.ts(1 hunks)src/express/zod.test.ts(2 hunks)src/express/zod.ts(1 hunks)src/fetch/validation.ts(1 hunks)src/utils.ts(1 hunks)src/valibot/index.ts(3 hunks)src/zod/index.ts(3 hunks)
🔇 Additional comments (15)
src/express/zod.ts (1)
66-67: LGTM! Verify response validation handling.
The separation of request validation from response validation improves the code's adherence to single responsibility principle. However, ensure that response validation is properly handled elsewhere in the codebase.
Let's verify the response validation implementation:
src/express/valibot.ts (1)
68-69: LGTM! Clean separation of request validation.
The destructuring of the validator into reqValidator makes it explicit that we're using request validation middleware. This aligns well with the single responsibility principle.
Let's verify that all usages of newValibotValidator have been updated to handle the new structure:
✅ Verification successful
All usages of newValibotValidator correctly handle the new structure
Based on the search results, all instances of newValibotValidator consistently use the destructured pattern to extract reqValidator:
src/express/valibot.ts:const { req: reqValidator } = newValibotValidator(pathMap);src/express/valibot.test.ts:const { req: reqValidator } = newValibotValidator(pathMap);
The same pattern is also consistently used with validatorMiddleware, which always receives the reqValidator. This confirms that the codebase has been properly updated to handle the new validator structure.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Check for any direct usage of newValibotValidator that might need updating
# Expected: All usages should destructure the return value
# Search for newValibotValidator usage
rg "newValibotValidator" -A 2 -B 2
# Search for validatorMiddleware usage to ensure consistent pattern
rg "validatorMiddleware" -A 2 -B 2
Length of output: 5481
examples/simple/withValidation.ts (2)
15-15: Consider uncommenting and utilizing the type annotation
The commented type ToApiEndpoints could provide better type safety and IDE support if utilized.
26-26: Consider utilizing type annotations for better type safety
The commented type annotations suggest potential type safety improvements. Consider implementing them if FetchT is available.
Also applies to: 42-42
package.json (1)
17-18: LGTM: New example script follows existing patterns
The new validation example script follows the established pattern of other example scripts and uses the appropriate executor (tsx).
src/core/spec.ts (1)
Line range hint 41-79: Verify integration with validation utilities.
Let's ensure these new types are properly integrated with the validation framework.
✅ Verification successful
Let me check the actual usage and implementation in the validation file to make a proper assessment.
Let me check how these types are used in the validation function to understand their integration better.
Types are properly integrated with validation system
The verification shows that the new types are correctly integrated into the validation framework:
UnknownApiEndpointsis properly imported and used in the validation function's type signature- The types are used in a higher-order function that wraps fetch calls with validation
- The validation system correctly uses these types to:
- Generate validators based on endpoint specifications
- Validate both requests and responses
- Handle validation errors according to the specified policy
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Check integration points with validation utilities
# Check for validation utility imports and usage
echo "Checking validation utility integration..."
rg -l "UnknownApiSpec|UnknownApiEndpoints" --type ts
# Check for potential validation middleware usage
echo "Checking validation middleware usage..."
ast-grep --pattern 'function $_(req: $_, res: $_) {
$$$
UnknownApiSpec
$$$
}'
Length of output: 365
Script:
#!/bin/bash
# Check the implementation details in validation.ts
echo "Checking validation.ts implementation..."
rg -A 10 "UnknownApiSpec|UnknownApiEndpoints" src/fetch/validation.ts
# Check for any type imports of these types
echo "Checking type imports..."
rg "import.*\{.*(?:UnknownApiSpec|UnknownApiEndpoints).*\}" --type ts
# Look for any validation functions or utilities that might use these types
echo "Checking validation functions..."
ast-grep --pattern 'function $name($params) {
$$$
(UnknownApiSpec|UnknownApiEndpoints)
$$$
}'
Length of output: 1238
Script:
#!/bin/bash
# Get the full validation function implementation
echo "Checking validation function implementation..."
rg -B 5 -A 20 "Endpoints extends UnknownApiEndpoints" src/fetch/validation.ts
# Check for any validator implementations
echo "Checking validator implementations..."
ast-grep --pattern 'const $_ = ($params: $_) => {
$$$
}' src/fetch/validation.ts
# Check for any response validator usage
echo "Checking response validator usage..."
rg -A 5 "responseValidatorGenerator" src/fetch/validation.ts
Length of output: 2092
src/express/zod.test.ts (1)
54-55: LGTM! Clear validator middleware initialization.
The destructuring of reqValidator from newZodValidator result improves code clarity by explicitly showing which part of the validator is being used.
src/express/valibot.test.ts (1)
61-62: LGTM! Good separation of request validation concerns.
The restructuring of the validator initialization improves code organization by explicitly handling request validation. This change aligns well with the PR's objective of enhancing client validation while maintaining existing functionality.
Let's verify the new validator structure:
src/zod/index.ts (1)
10-11: Imports are appropriate
The imported modules from "../core/validate" are correctly added and utilized.
src/fetch/validation.ts (6)
16-22: Function parseURL is well implemented
The parseURL function correctly parses the URL string with appropriate error handling, ensuring robustness.
24-31: Function headersToRecord is efficient and accurate
The headersToRecord function effectively converts HTTP headers into a record format, maintaining the correct key-value pairs.
75-92: Function newErrorHandler handles policies effectively
The implementation correctly handles different error policies (throw or log) and uses the unreachable function to ensure exhaustive checks.
93-110: Function newResponseErrorHandler mirrors newErrorHandler appropriately
The response error handler effectively mirrors the structure of newErrorHandler, ensuring consistent error handling for responses.
158-165: Class ValidateError is well-defined
The ValidateError class extends the Error class appropriately, encapsulating validation errors with a clear message and accessible properties.
167-186: Functions handleValidatorsError and handleResponseValidatorsError effectively manage errors
These functions systematically check for errors in validation results and invoke callbacks when errors are encountered, ensuring consistent error handling.
| export const memoize = <T extends (...args: any[]) => any>(fn: T): T => { | ||
| const cache: Record<string, ReturnType<T>> = {}; | ||
| return ((...args: Parameters<T>) => { | ||
| const key = JSON.stringify(args); | ||
| if (cache[key] === undefined) { | ||
| cache[key] = fn(...args); | ||
| } | ||
| return cache[key]; | ||
| }) as T; | ||
| }; |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Consider enhancing the memoize function's robustness
The current implementation has several potential issues:
-
Using
JSON.stringifyfor cache keys:- Cannot handle functions, undefined, or circular references
- Sensitive to object property order
- Different object structures with same values generate different keys
-
No cache management strategy could lead to memory leaks
Consider this improved implementation:
-export const memoize = <T extends (...args: any[]) => any>(fn: T): T => {
+export const memoize = <T extends (...args: unknown[]) => unknown>(
+ fn: T,
+ options: { maxSize?: number; keyGenerator?: (...args: Parameters<T>) => string } = {}
+): T => {
const cache: Record<string, ReturnType<T>> = {};
+ const keys: string[] = [];
+ const { maxSize = 1000, keyGenerator = (...args) => JSON.stringify(args) } = options;
return ((...args: Parameters<T>) => {
- const key = JSON.stringify(args);
+ const key = keyGenerator(...args);
if (cache[key] === undefined) {
+ if (keys.length >= maxSize) {
+ const oldestKey = keys.shift();
+ if (oldestKey) delete cache[oldestKey];
+ }
cache[key] = fn(...args);
+ keys.push(key);
}
return cache[key];
}) as T;
};This implementation:
- Adds cache size limits to prevent memory leaks
- Allows custom key generation for complex objects
- Uses more type-safe parameters
📝 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.
| export const memoize = <T extends (...args: any[]) => any>(fn: T): T => { | |
| const cache: Record<string, ReturnType<T>> = {}; | |
| return ((...args: Parameters<T>) => { | |
| const key = JSON.stringify(args); | |
| if (cache[key] === undefined) { | |
| cache[key] = fn(...args); | |
| } | |
| return cache[key]; | |
| }) as T; | |
| }; | |
| export const memoize = <T extends (...args: unknown[]) => unknown>( | |
| fn: T, | |
| options: { maxSize?: number; keyGenerator?: (...args: Parameters<T>) => string } = {} | |
| ): T => { | |
| const cache: Record<string, ReturnType<T>> = {}; | |
| const keys: string[] = []; | |
| const { maxSize = 1000, keyGenerator = (...args) => JSON.stringify(args) } = options; | |
| return ((...args: Parameters<T>) => { | |
| const key = keyGenerator(...args); | |
| if (cache[key] === undefined) { | |
| if (keys.length >= maxSize) { | |
| const oldestKey = keys.shift(); | |
| if (oldestKey) delete cache[oldestKey]; | |
| } | |
| cache[key] = fn(...args); | |
| keys.push(key); | |
| } | |
| return cache[key]; | |
| }) as T; | |
| }; |
| } catch (e: unknown) { | ||
| if (e instanceof ValidateError) { | ||
| console.log("error thrown", (e.error as ZodError).format()); | ||
| } | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Enhance error handling
The error handling could be more specific and informative:
- Add handling for network errors
- Add handling for JSON parsing errors
- Log the full error stack in development
} catch (e: unknown) {
if (e instanceof ValidateError) {
console.log("error thrown", (e.error as ZodError).format());
+ } else if (e instanceof TypeError) {
+ console.error("Network error:", e.message);
+ } else {
+ console.error("Unexpected error:", e);
}
}📝 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.
| } catch (e: unknown) { | |
| if (e instanceof ValidateError) { | |
| console.log("error thrown", (e.error as ZodError).format()); | |
| } | |
| } | |
| } catch (e: unknown) { | |
| if (e instanceof ValidateError) { | |
| console.log("error thrown", (e.error as ZodError).format()); | |
| } else if (e instanceof TypeError) { | |
| console.error("Network error:", e.message); | |
| } else { | |
| console.error("Unexpected error:", e); | |
| } | |
| } |
| if (!response.ok) { | ||
| const { message } = await response.json(); | ||
| return console.error(message); | ||
| } | ||
| const { names } = await response.json(); | ||
| console.log(names); |
There was a problem hiding this comment.
Avoid parsing response body twice
The response body is being parsed twice: once for error checking and once for data extraction. This can lead to errors as the body stream can only be consumed once.
- if (!response.ok) {
- const { message } = await response.json();
- return console.error(message);
- }
- const { names } = await response.json();
+ const data = await response.json();
+ if (!response.ok) {
+ return console.error(data.message);
+ }
+ const { names } = data;📝 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.
| if (!response.ok) { | |
| const { message } = await response.json(); | |
| return console.error(message); | |
| } | |
| const { names } = await response.json(); | |
| console.log(names); | |
| const data = await response.json(); | |
| if (!response.ok) { | |
| return console.error(data.message); | |
| } | |
| const { names } = data; | |
| console.log(names); |
| "author": "mpppk", | ||
| "license": "ISC", | ||
| "devDependencies": { | ||
| "@types/path-to-regexp": "^1.7.0", |
There was a problem hiding this comment.
💡 Codebase verification
Update @types/path-to-regexp to match path-to-regexp version
The type definitions package @types/path-to-regexp@1.7.0 is indeed outdated and doesn't match the runtime dependency path-to-regexp@8.2.0. However, after verification:
- Latest path-to-regexp version is 8.2.0
- Latest @types/path-to-regexp version is 1.7.0
- No newer versions of @types/path-to-regexp are available
Since path-to-regexp v8.x.x now includes built-in TypeScript type definitions, the separate @types package is no longer needed. You should remove the @types/path-to-regexp dependency entirely.
🔗 Analysis chain
Fix version mismatch between @types/path-to-regexp and path-to-regexp
The type definitions version (@types/path-to-regexp@^1.7.0) doesn't match the runtime dependency version (path-to-regexp@^8.2.0). This mismatch could lead to type definition inconsistencies.
Let's verify the latest compatible versions:
Consider updating to matching major versions to ensure type safety.
Also applies to: 123-123
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Check latest versions of path-to-regexp and its type definitions
echo "Latest path-to-regexp version:"
npm view path-to-regexp version
echo "\nLatest @types/path-to-regexp version:"
npm view @types/path-to-regexp version
echo "\nVerifying version compatibility..."
npm view @types/path-to-regexp@* version
Length of output: 380
| const method = maybeMethod?.toLowerCase(); | ||
| if (!isMethod(method)) { | ||
| return Result.error(newValidatorMethodNotFoundError(method)); | ||
| } | ||
| return getApiSpec(endpoints, path, method); | ||
| }; |
There was a problem hiding this comment.
Handle potential undefined value for method in preCheck.
If maybeMethod is undefined, the method variable becomes undefined, and passing it to isMethod or newValidatorMethodNotFoundError may cause unintended behavior or runtime errors. Ensure that method is properly defined before proceeding with validation.
Consider updating the code to handle undefined values for method:
const method = maybeMethod?.toLowerCase();
+ if (!method) {
+ return Result.error(newValidatorMethodNotFoundError('undefined'));
+ }
if (!isMethod(method)) {
return Result.error(newValidatorMethodNotFoundError(method));
}📝 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.
| const method = maybeMethod?.toLowerCase(); | |
| if (!isMethod(method)) { | |
| return Result.error(newValidatorMethodNotFoundError(method)); | |
| } | |
| return getApiSpec(endpoints, path, method); | |
| }; | |
| const method = maybeMethod?.toLowerCase(); | |
| if (!method) { | |
| return Result.error(newValidatorMethodNotFoundError('undefined')); | |
| } | |
| if (!isMethod(method)) { | |
| return Result.error(newValidatorMethodNotFoundError(method)); | |
| } | |
| return getApiSpec(endpoints, path, method); | |
| }; |
| V extends (input: ValidatorsInput) => { | ||
| validator: AnyValidators; | ||
| error: unknown; | ||
| }, |
There was a problem hiding this comment.
Handle the error returned by the validator function.
The validator function now returns an object containing both validator and error. Currently, the error is not being used or handled, which may lead to unhandled exceptions or missed error reporting.
Consider handling the error appropriately, such as logging it or passing it to the next function to trigger Express's error handling middleware.
| const newPathMather = <E extends Record<string, unknown>>(endpoints: E) => { | ||
| const mMatch = memoize(match); | ||
| return (path: string) => { | ||
| // FIXME matchedはendpointsのkeys | ||
| const ret: MatchResult[] = []; | ||
| for (const definedPath of Object.keys(endpoints)) { | ||
| const result = mMatch(definedPath)(path); | ||
| if (!result) { | ||
| continue; | ||
| } | ||
| ret.push({ | ||
| matched: definedPath, | ||
| // TODO: こんな適当にキャストしていいんだっけ? | ||
| params: result.params as Record<string, string | string[]>, | ||
| }); | ||
| } | ||
| return ret; | ||
| }; | ||
| }; |
There was a problem hiding this comment.
Resolve the TODO and FIXME comments in newPathMather function
The function contains TODO and FIXME comments that need your attention:
- Line 40:
// FIXME matchedはendpointsのkeys(FIXME:matchedshould be one of theendpointskeys) - Line 49:
// TODO: こんな適当にキャストしていいんだっけ?(TODO: Is it acceptable to cast like this?)
These comments indicate concerns about the correctness of the matched property and the safety of the type casting for params. Addressing these will enhance type safety and ensure the function behaves as intended.
Would you like assistance in resolving these issues or should I open a GitHub issue to track them?
| error: any; | ||
| }, |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Avoid using any type and remove ESLint disable comments
Using any reduces type safety. Instead of disabling ESLint rules, consider using unknown or a more specific type for the error parameter to enhance type safety.
Apply this diff to improve type safety:
- // eslint-disable-next-line @typescript-eslint/no-explicit-any
error: any;
+ error: unknown;Similarly, in line 137, remove the ESLint disable comment and ensure proper typing:
- // eslint-disable-next-line @typescript-eslint/no-explicit-any
const { validator, error } = validatorGenerator(vInput);Ensure that validatorGenerator returns an error of type unknown or a specific type to maintain type safety.
Also applies to: 137-138
| // TODO: jsonじゃない時どうするか | ||
| // TODO: response bodyを直接渡すのはおかしい | ||
| const responseValidator = responseValidatorGenerator({ |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Handle non-JSON responses and review passing response body
There are TODO comments indicating potential issues:
- Line 142: Handling responses that are not JSON.
- Line 143: It's questionable to pass the response body directly.
Consider implementing logic to handle various response content types and ensure that the response body is processed appropriately. This might involve checking the Content-Type header and parsing the response accordingly.
Would you like assistance in addressing these concerns?
| handleResponseError(runResponseValidators(responseValidator)); | ||
| // // TODO: レスポンスをvalidate | ||
| return res; |
There was a problem hiding this comment.
Implement response validation logic
A TODO comment indicates that response validation needs to be completed.
- Line 152:
// TODO: レスポンスをvalidate(TODO: Validate the response)
Ensure that the response is properly validated against the defined validators before returning it to the caller.
Would you like assistance in implementing the response validation?
Summary by CodeRabbit
Release Notes
New Features
Bug Fixes
Documentation
Chores