Auto-Initialize the API #23
Conversation
This allows developers to call the API without manually initializing. This does not prevent manual initializtion from occurring, nor does it override it. This also shows a proper error if the API cannot initialize, instead of showing non-informative failed API responses.
WalkthroughThe changes introduce a new optional property Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant API
participant Config
User->>API: Call API function
API->>Config: Check if INITIALIZED
alt Not Initialized
Config->>API: Call init()
API->>Config: Merge configuration
Config->>API: Set INITIALIZED to true
end
API->>API: Proceed with API request
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 (5)
readme.md (3)
31-34: Add a note about auto-initialization behavior.While the simplified example is good, it would be helpful to add a brief note explaining that the API will auto-initialize on the first request using environment variables. This helps users understand the behavior and requirements.
import { Users } from "@kinde/management-api-js"; +// The API will auto-initialize on first request using environment variables const { users } = await Users.getUsers();
48-58: Enhance the manual initialization section with more context.While the example is clear, it would be beneficial to:
- Explain when manual initialization might be preferred over auto-initialization
- Clarify how manual initialization relates to environment variables
- Note that manual initialization takes precedence over auto-initialization
### Manually Initializing the API +Manual initialization allows you to: +- Override environment variables with different credentials +- Initialize the API before any requests are made +- Have more control over the initialization timing + +Note: Manual initialization takes precedence over auto-initialization. + ```js import { init } from "@kinde/management-api-js";
31-31: Add section about error handling improvements.The PR introduces improved error handling with clearer messages when API initialization fails. Consider adding a section about common errors and their meanings to help developers with debugging.
+## Error Handling + +The SDK now provides clearer error messages when initialization fails. Common errors include: +- "Environment variables not set": Check that KINDE_DOMAIN, KINDE_MANAGEMENT_CLIENT_ID, and KINDE_MANAGEMENT_CLIENT_SECRET are properly configured +- "Invalid credentials": Verify that your client ID and secret are correct + import { Users } from "@kinde/management-api-js";lib/config.ts (1)
Line range hint
1-63: Consider enhancing security and validationThe current implementation could benefit from additional security measures and validation:
- Sensitive configuration values should be redacted in logs
- Additional validation for required fields (clientId, clientSecret, audience)
- The TokenStore interface could be better utilized for token management
Consider implementing these improvements:
- Add input validation for all required fields
- Implement secure logging that redacts sensitive information
- Add TypeScript strict type checking for configuration objects
Would you like me to provide a detailed implementation for any of these suggestions?
lib/api/core/request.ts (1)
Line range hint
345-351: Update JSDoc to document auto-initialization behavior.The function documentation should be updated to reflect the auto-initialization feature and its implications.
Consider updating the JSDoc:
/** * Request method * @param config The OpenAPI configuration object * @param options The request options from the service + * @description This method automatically initializes the API if not already initialized. + * The initialization is thread-safe and handles concurrent requests properly. + * Manual initialization is still supported and will be respected if performed. * @returns CancelablePromise<T> - * @throws ApiError + * @throws ApiError When the request fails or initialization fails */
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (4)
- lib/api/core/OpenAPI.ts (2 hunks)
- lib/api/core/request.ts (2 hunks)
- lib/config.ts (1 hunks)
- readme.md (1 hunks)
🔇 Additional comments (4)
lib/api/core/OpenAPI.ts (2)
57-57: LGTM! Clean implementation of initialization tracking.The default value of
falsealigns perfectly with the lazy initialization strategy, allowing for both automatic and manual initialization paths while maintaining backward compatibility.
40-40: Verify if INITIALIZED should be marked as optional.The
INITIALIZEDproperty inOpenAPIConfigtype is not marked as optional. While this ensures type safety by requiring the property to be present, consider if it should be marked as optional (INITIALIZED?: boolean) to maintain flexibility for partial configurations.✅ Verification successful
The INITIALIZED property should remain non-optional
The implementation shows that
INITIALIZEDis a critical flag used for ensuring proper initialization state. Inrequest.ts, there's an explicit checkif (!config.INITIALIZED)that triggers initialization, making this a required runtime safeguard. Making it optional would weaken this type safety mechanism and go against the PR's objective of reliable initialization tracking.Key evidence:
- Used in
request<T>()function for mandatory initialization check- Defined as a core property in the main
OpenAPIconstant- Serves as a required runtime safeguard across the codebase
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Check if there are any existing implementations of OpenAPIConfig # that might break with this non-optional property # Search for OpenAPIConfig implementations rg -l "implements.*OpenAPIConfig|:.*OpenAPIConfig|as.*OpenAPIConfig"Length of output: 118
Script:
#!/bin/bash # Let's examine the actual implementations to understand how OpenAPIConfig is used rg -A 5 -B 5 "implements.*OpenAPIConfig|:.*OpenAPIConfig|as.*OpenAPIConfig" lib/api/core/OpenAPI.ts lib/api/core/request.tsLength of output: 2899
readme.md (1)
39-46: LGTM!The example clearly demonstrates how to pass parameters while maintaining consistency with the auto-initialization approach.
lib/api/core/request.ts (1)
352-356:⚠️ Potential issueAdd synchronization and error handling for initialization.
The current implementation has potential issues:
- Race condition: Multiple concurrent requests could trigger initialization simultaneously
- Silent failures: No error handling for init() failures
- Side effects: Initialization effects are not properly managed
Consider implementing this safer approach:
// Verify that config has been initialized - if (!config.INITIALIZED) { - init(); - } + if (!config.INITIALIZED) { + try { + // Use initialization lock to prevent race conditions + if (!config.initializationPromise) { + config.initializationPromise = init(); + } + await config.initializationPromise; + } catch (error) { + throw new ApiError( + options, + { status: 500, ok: false }, + `API initialization failed: ${error.message}` + ); + } + }This change:
- Prevents race conditions using a promise-based lock
- Properly handles initialization failures
- Maintains consistent error reporting
Let's verify the initialization handling in the codebase:
|
|
||
| // Mark OpenAPI as initialized | ||
| OpenAPI.INITIALIZED = true; |
There was a problem hiding this comment.
Consider improving initialization safety and documentation
The initialization tracking addition needs some improvements:
- The function should be synchronized to prevent race conditions during parallel initialization attempts
- The INITIALIZED flag should only be set after ensuring all initialization steps succeed
- The JSDoc should document the initialization state behavior
Here's a suggested implementation:
/**
* Initializes the integration with Kinde, using either provided configuration
* or default values from `kindeConfig`.
*
* @param {Object} [config] - Optional configuration object for Kinde.
* @param {string} [config.kindeDomain] - The domain of your Kinde account.
* @param {string} [config.clientId] - Your Kinde M2M client ID.
* @param {string} [config.clientSecret] - Your Kinde M2M client secret.
+ * @throws {Error} If initialization fails due to missing configuration or invalid state
+ * @returns {Promise<void>} - Resolves when initialization is complete
*/
-export const init = (
+export const init = async (
config: Pick<
configType,
"kindeDomain" | "clientId" | "clientSecret"
> = kindeConfig,
-) => {
+) => {
+ // Prevent parallel initialization
+ if (OpenAPI.INITIALIZED) {
+ return;
+ }
if (!process.env.KINDE_DOMAIN && config.kindeDomain === "") {
throw new Error("kindeDomain or env KINDE_DOMAIN is not set");
}
_merge(
kindeConfig,
{
clientId: process.env.KINDE_MANAGEMENT_CLIENT_ID,
clientSecret: process.env.KINDE_MANAGEMENT_CLIENT_SECRET,
audience: process.env.KINDE_DOMAIN + "/api",
kindeDomain: process.env.KINDE_DOMAIN,
},
config,
);
_merge(OpenAPI, {
BASE: kindeConfig.kindeDomain,
TOKEN: async () => {
return await getToken();
},
});
+ try {
+ // Verify the configuration by attempting to get an initial token
+ await getToken();
+ OpenAPI.INITIALIZED = true;
+ } catch (error) {
+ throw new Error(`Failed to initialize: ${error.message}`);
+ }
};📝 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.
| // Mark OpenAPI as initialized | |
| OpenAPI.INITIALIZED = true; | |
| /** | |
| * Initializes the integration with Kinde, using either provided configuration | |
| * or default values from `kindeConfig`. | |
| * | |
| * @param {Object} [config] - Optional configuration object for Kinde. | |
| * @param {string} [config.kindeDomain] - The domain of your Kinde account. | |
| * @param {string} [config.clientId] - Your Kinde M2M client ID. | |
| * @param {string} [config.clientSecret] - Your Kinde M2M client secret. | |
| * @throws {Error} If initialization fails due to missing configuration or invalid state | |
| * @returns {Promise<void>} - Resolves when initialization is complete | |
| */ | |
| export const init = async ( | |
| config: Pick< | |
| configType, | |
| "kindeDomain" | "clientId" | "clientSecret" | |
| > = kindeConfig, | |
| ) => { | |
| // Prevent parallel initialization | |
| if (OpenAPI.INITIALIZED) { | |
| return; | |
| } | |
| if (!process.env.KINDE_DOMAIN && config.kindeDomain === "") { | |
| throw new Error("kindeDomain or env KINDE_DOMAIN is not set"); | |
| } | |
| _merge( | |
| kindeConfig, | |
| { | |
| clientId: process.env.KINDE_MANAGEMENT_CLIENT_ID, | |
| clientSecret: process.env.KINDE_MANAGEMENT_CLIENT_SECRET, | |
| audience: process.env.KINDE_DOMAIN + "/api", | |
| kindeDomain: process.env.KINDE_DOMAIN, | |
| }, | |
| config, | |
| ); | |
| _merge(OpenAPI, { | |
| BASE: kindeConfig.kindeDomain, | |
| TOKEN: async () => { | |
| return await getToken(); | |
| }, | |
| }); | |
| try { | |
| // Verify the configuration by attempting to get an initial token | |
| await getToken(); | |
| OpenAPI.INITIALIZED = true; | |
| } catch (error) { | |
| throw new Error(`Failed to initialize: ${error.message}`); | |
| } | |
| }; |
|
Hi @cainspencerm, Thanks for raising this. Unfortunately the code under The initial implementation of this package didn't have an
|
|
Ah, I assumed incorrectly that |
|
Are you suggesting that the current changes on this pull request wouldn't work with Nuxt? If it could work, what do you think about tacking in a script at the end of Then, |
This allows developers to call the API without manually initializing. This does not prevent manual initialization from occurring, nor does it override it. This also shows a proper error if the API cannot initialize, instead of showing non-informative failed API responses.
Additions
Initialization Tracking
A boolean flag is added to
OpenAPI(and its typeOpenAPIConfig). The flag defaults to false, entertaining a lazy auto-initialization strategy. This is a better initialization strategy than immediate initialization, since it gives the developer the option to initialize manually before using the API. Further, lazy auto-initialization prevents creating a token unnecessarily, should the developer choose to manually initialize.Auto-Initialization on Request
When
requestis called (aka__requestas used in the generated API file), initialization is verified via the boolean flag. If the configuration has not yet been initialized, initialization occurs. The request then proceeds as normal. This impacts performance minimally, as it merely shifts the time of initialization to the beginning of the first request and adds a single boolean check otherwise.Benefits
The current state of the package (v0.9.0) causes silent and misleading failures surrounding initialization. If
initis not called before the first API request, the package will throw anInvalid URLerror due to the default URL valuehttps://{your_subdomain}.kinde.com. This doesn't suggest that the environment isn't correctly set up, nor does it suggest thatinithas not yet been called.With auto-initialization,
initwill be called regardless, leading to more informed debugging when the environment is not set up properly. Instead ofInvalid URL, developers might seekindeDomain or env KINDE_DOMAIN is not set.Further,
initis boilerplate for developers that store the domain and other parameters in the environment. It is helpful to the development process to handle it in the background. However, leaving it available for manual initialization when needed is good for the manually-set use case and backwards compatibility.Checklist