Add poller logic to check the status of the provisioning job

[saikambaiyyagari]

Description

f2db577 Add poller logic to check the status of the provisioning job

Detail

Checklist

• 💂‍♂️ includes new unit and functional tests

[Copilot]

Pull request overview

This PR adds polling logic to check the status of connector provisioning jobs after they are created and uploaded. The implementation introduces a new poller module that polls the provisioning status endpoint with adaptive polling intervals and exponential backoff retry logic for HTTP errors.

Changes:

• Added a new poller module with polling logic, HTTP retry mechanism, and adaptive polling intervals
• Updated the createConnector function to return a jobId from the API response
• Modified the publish command to poll for provisioning status after upload and handle SUCCESS, FAILED, and ABORTED states
• Added comprehensive unit and functional tests for the new polling functionality

Reviewed changes

Copilot reviewed 6 out of 6 changed files in this pull request and generated no comments.

Show a summary per file

File	Description
packages/zcli-connectors/src/lib/publish/poller.ts	New module implementing polling logic with adaptive intervals and HTTP retry mechanism
packages/zcli-connectors/src/lib/publish/poller.test.ts	Comprehensive unit tests for polling logic covering success, failure, retry, and timeout scenarios
packages/zcli-connectors/src/lib/publish/publish.ts	Updated createConnector to return jobId from API response
packages/zcli-connectors/src/lib/publish/publish.test.ts	Updated tests to include jobId in mock responses
packages/zcli-connectors/src/commands/connectors/publish.ts	Integrated polling into publish flow with status-specific error handling
packages/zcli-connectors/tests/functional/publish.test.ts	Added functional tests for SUCCESS, FAILED, and ABORTED provisioning states

Comments suppressed due to low confidence (11)

packages/zcli-connectors/src/lib/publish/poller.ts:96

• There are two different error messages for timeout errors that differ only by punctuation: 'Provisioning status polling timed out after 5 minutes' (line 80) and 'Provisioning status polling timed out after 5 minutes.' (line 96, with a period). This inconsistency could cause confusion. Consider using the same message for consistency.

      throw new Error('Provisioning status polling timed out after 5 minutes')
    }

    try {
      const response = await makeRequestWithRetry(endpoint)
      const statusResponse: ProvisioningStatusResponse = response.data
      const status = statusResponse.status

      if (status === 'SUCCESS' || status === 'FAILED' || status === 'ABORTED') {
        return { status, reason: statusResponse.reason }
      }

      const pollInterval = getAdaptivePollIntervalMs(startTime)
      await new Promise(resolve => setTimeout(resolve, pollInterval))
    } catch (error) {
      if (error instanceof Error && error.message.includes('timed out')) {
        throw new Error('Provisioning status polling timed out after 5 minutes.')

packages/zcli-connectors/src/commands/connectors/publish.ts:115

• The reason field can be undefined (as defined in the return type of pollProvisioningStatus), but it's being directly interpolated into error messages at lines 112 and 115. When reason is undefined, this will result in error messages like "Connector provisioning failed: undefined". Consider using a fallback message such as reason || 'No reason provided' or reason ?? 'Unknown reason' to provide a more user-friendly error message.

        throw new Error(`Connector provisioning failed: ${reason}`)
      } else if (finalStatus === 'ABORTED') {
        spinner.fail(chalk.yellow('Connector provisioning was aborted'))
        throw new Error(`Connector provisioning was aborted: ${reason}`)

packages/zcli-connectors/src/lib/publish/poller.ts:90

• There's no validation to ensure the status returned from the API is one of the expected ProvisioningStatus values. If the API returns an unexpected status value (e.g., 'UNKNOWN' or any other string not in the type definition), the polling loop will continue indefinitely until timeout. Consider adding validation to check if the status is a valid ProvisioningStatus value and handle unexpected statuses appropriately.

      const status = statusResponse.status

      if (status === 'SUCCESS' || status === 'FAILED' || status === 'ABORTED') {
        return { status, reason: statusResponse.reason }
      }

packages/zcli-connectors/src/lib/publish/publish.ts:88

• The code assumes that response.data.job_id will always be present, but there's no validation to ensure it exists before returning it. If the API response is missing the job_id field, this will result in jobId being undefined, which could cause issues downstream in pollProvisioningStatus. Consider adding validation to check if job_id exists in the response and throw a descriptive error if it's missing.

  return {
    uploadUrl: response.data.upload_url,
    connectorName,
    jobId: response.data.job_id
  }

packages/zcli-connectors/src/lib/publish/poller.ts:84

• The timeout check at line 79 only occurs at the beginning of each polling iteration, before calling makeRequestWithRetry. If makeRequestWithRetry takes a long time (potentially up to 45 seconds with 3 retries at 5s, 10s, and 20s delays), the actual timeout could exceed the 5-minute limit. Consider checking the elapsed time after makeRequestWithRetry completes as well, or moving the timeout check into a more frequently executed position.

    const timeElapsed = Date.now() - startTime
    if (timeElapsed >= POLLING_TIMEOUT_MS) {
      throw new Error('Provisioning status polling timed out after 5 minutes')
    }

    try {
      const response = await makeRequestWithRetry(endpoint)

packages/zcli-connectors/src/lib/publish/poller.ts:73

• Consider extracting the return type { status: ProvisioningStatus, reason?: string } to a separate exported interface (e.g., ProvisioningResult). This would make the code more maintainable and allow consumers to type their variables more clearly when calling this function. Currently, the return type is defined inline which makes it harder to reference elsewhere.

export async function pollProvisioningStatus (
  connectorName: string,
  jobId: string
): Promise<{ status: ProvisioningStatus, reason?: string }> {

packages/zcli-connectors/src/lib/publish/poller.ts:96

• The error handling at line 95 checks if the error message 'includes' the text 'timed out', which is a very generic check. This could accidentally match HTTP timeout errors from the requestAPI call itself (like network timeouts), not just the polling timeout. Consider checking for a more specific condition or error type to distinguish between polling timeout and HTTP timeout errors.

      if (error instanceof Error && error.message.includes('timed out')) {
        throw new Error('Provisioning status polling timed out after 5 minutes.')

packages/zcli-connectors/src/lib/publish/poller.ts:54

• The comment at line 53 is misaligned after the case statement. It should either be moved to line 52 (before the case) or indented to align with the return statement on line 54 for better readability.

  case timeElapsed < 60000:
  // every 5s for first 1min
    return 5000

packages/zcli-connectors/src/lib/publish/poller.test.ts:22

• The test at line 22 is testing the edge case where timeElapsed equals exactly 60000ms. However, the switch statement checks timeElapsed < 60000 which returns false when timeElapsed is exactly 60000. Consider adding a test case for exactly 59999ms to verify the boundary condition is working as expected, or adjust the conditions to use <= if the intent is to include the boundary in the first interval.

        expect(getAdaptivePollIntervalMs(now - 60000)).to.equal(30000)

packages/zcli-connectors/src/lib/publish/poller.ts:18

• The return type of makeRequestWithRetry is declared as Promise<any>. For better type safety, consider defining a more specific return type that matches the structure of the response object (e.g., { status: number; data: any }), or at least document what the expected return structure is.

async function makeRequestWithRetry (endpoint: string): Promise<any> {

packages/zcli-connectors/tests/functional/publish.test.ts:188

• The functional tests don't cover the scenario where pollProvisioningStatus throws an error (e.g., timeout error, HTTP error after retries). While there are tests for FAILED and ABORTED states, there's no test for when the polling itself fails with an exception. Consider adding a test case to ensure the command properly handles and reports polling errors.

    it('should handle publish connector provisioned with failed state', async () => {
      sinon.stub(validations, 'runValidationChecks').resolves()
      sinon.stub(publishModule, 'createConnector').resolves(
        {
          uploadUrl: 'https://example.com/upload',
          connectorName: 'test-connector',
          jobId: 'job-123'
        }
      )
      sinon.stub(publishModule, 'uploadConnectorPackage').resolves()
      sinon.stub(pollerModule, 'pollProvisioningStatus').resolves({ status: 'FAILED', reason: 'Provisioning failed' })

      try {
        await publishCommand.run()
      } catch (error) {
        caughtError = error as Error
      }

      expect(caughtError).to.exist
      expect(caughtError?.message).to.include('Connector provisioning failed:')
    })

    it('should handle publish connector provisioned with aborted state', async () => {
      sinon.stub(validations, 'runValidationChecks').resolves()
      sinon.stub(publishModule, 'createConnector').resolves(
        {
          uploadUrl: 'https://example.com/upload',
          connectorName: 'test-connector',
          jobId: 'job-123'
        }
      )
      sinon.stub(publishModule, 'uploadConnectorPackage').resolves()
      sinon.stub(pollerModule, 'pollProvisioningStatus').resolves({ status: 'ABORTED', reason: 'Provisioning aborted' })

      try {
        await publishCommand.run()
      } catch (error) {
        caughtError = error as Error
      }

      expect(caughtError).to.exist
      expect(caughtError?.message).to.include('Connector provisioning was aborted')
    })

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.