Conversation
|
@copilot Can you finish the TODOs here? |
There was a problem hiding this comment.
Pull Request Overview
This PR adds a new waitForCertificates method to the KeetaKYCVerification class that polls for certificate availability with configurable timeout and interval parameters.
- Introduces polling mechanism for certificate retrieval
- Adds configurable timeout (default 10 minutes) and poll interval (default 500ms)
| for (const startTime = Date.now(); Date.now() - startTime < timeout; ) { | ||
| try { | ||
| return(await this.getCertificates()); | ||
| } catch (getCertificatesError) { | ||
| /* XXX:TODO */ |
There was a problem hiding this comment.
The error handling immediately re-throws all errors, preventing any polling behavior. The method should only re-throw non-recoverable errors and sleep before retrying on expected failures (e.g., certificate not ready yet). The pollInterval parameter should be used to implement the delay between retries.
| for (const startTime = Date.now(); Date.now() - startTime < timeout; ) { | |
| try { | |
| return(await this.getCertificates()); | |
| } catch (getCertificatesError) { | |
| /* XXX:TODO */ | |
| const startTime = Date.now(); | |
| while (Date.now() - startTime < timeout) { | |
| try { | |
| return(await this.getCertificates()); | |
| } catch (getCertificatesError: any) { | |
| // Only retry on expected recoverable errors (e.g., certificate not ready yet) | |
| // Customize this check as needed for your error types | |
| if ( | |
| getCertificatesError?.code === 'CERTIFICATE_NOT_READY' || | |
| (getCertificatesError?.message && getCertificatesError.message.includes('not ready')) || | |
| getCertificatesError?.status === 404 // Example: 404 means not found yet | |
| ) { | |
| // Wait before retrying | |
| await new Promise(resolve => setTimeout(resolve, pollInterval)); | |
| continue; | |
| } | |
| // Non-recoverable error, re-throw |
| for (const startTime = Date.now(); Date.now() - startTime < timeout; ) { | ||
| try { | ||
| return(await this.getCertificates()); | ||
| } catch (getCertificatesError) { | ||
| /* XXX:TODO */ | ||
| throw(getCertificatesError); |
There was a problem hiding this comment.
The for loop will execute continuously without any delay between iterations, potentially causing high CPU usage and unnecessary API calls. After a failed attempt, the code should wait for 'pollInterval' milliseconds before retrying.
| for (const startTime = Date.now(); Date.now() - startTime < timeout; ) { | |
| try { | |
| return(await this.getCertificates()); | |
| } catch (getCertificatesError) { | |
| /* XXX:TODO */ | |
| throw(getCertificatesError); | |
| const startTime = Date.now(); | |
| while (Date.now() - startTime < timeout) { | |
| try { | |
| return(await this.getCertificates()); | |
| } catch (getCertificatesError) { | |
| // Wait for pollInterval before retrying | |
| await new Promise(resolve => setTimeout(resolve, pollInterval)); |
This change adds a
waitForCertificatesutility method to the KYC client to handle waiting for the certificate from the service provider.TODO: