Refresh connection flow

[pellicceama]

Important

Enhance OAuth connection flow with improved error handling, schema updates, and UI modifications for better reconnection logic.

• Behavior:
  • Adds error handling for refresh_token_external_error in nangoProxyLink.ts and remote-procedure.ts.
  • Updates performConnectionCheck in connectionRouter.ts to set connection status to 'disconnected' on refresh token error.
  • Modifies listConnections in pipelineRouter.ts to perform connection checks.
• Schemas:
  • Adds error field to nangoConnectionWithCredentials in nangoProxyLink.ts.
  • Adds comments for potential error fields in oauthConnector.ts.
• UI Components:
  • Adds onReconnect prop to ConnectionCard in ConnectionCard.tsx.
  • Updates ConnectionsTabContent and ConnectionPortal to handle reconnection logic.
  • Removes refetch call on success in AddConnectionTabContent.tsx.
• Misc:
  • Adds Cloudflare-related entries to .gitignore.

^{This description was created by for 0afc98a. It will automatically update as commits are pushed.}

[vercel]

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name	Status	Preview	Comments	Updated (UTC)
openint	✅ Ready (Inspect)	Visit Preview	💬 Add feedback	Dec 30, 2024 2:47pm

[ellipsis-dev]

👍 Looks good to me! Reviewed everything up to cddefc1 in 22 seconds

More details

• Looked at 310 lines of code in 10 files
• Skipped 0 files when reviewing.
• Skipped posting 5 drafted comments based on config settings.

1. kits/cdk/internal/remote-procedure.ts:69

• Draft comment:
  Redundant error logging and rethrowing. Consider removing the catch block to simplify the code.
• Reason this comment was not posted:
  Confidence changes required: 50%
  The code in remote-procedure.ts has a redundant catch block that logs the error and then rethrows it. This is unnecessary and can be removed to simplify the code.

2. packages/engine-backend/router/connectionRouter.ts:55

• Draft comment:
  Consider refactoring the repeated condition remoteCtx.remote.settings?.oauth?.error into a separate function to improve code readability and maintainability. This condition is used in multiple places.
• Reason this comment was not posted:
  Confidence changes required: 50%
  In connectionRouter.ts, the performConnectionCheck function has a condition that checks for remoteCtx.remote.settings?.oauth?.error. This condition is repeated in multiple places and can be refactored into a separate function to improve code readability and maintainability.

3. packages/engine-frontend/components/ConnectionPortal.tsx:126

• Draft comment:
  The onReconnect function is commented out. If not needed, remove it to clean up the code. If needed, implement it properly.
• Reason this comment was not posted:
  Confidence changes required: 50%
  In ConnectionPortal.tsx, the onReconnect function is defined but commented out. If it's not needed, it should be removed to clean up the code. If it is needed, it should be properly implemented.

4. packages/engine-frontend/components/AddConnectionTabContent.tsx:35

• Draft comment:
  The refetch function call is commented out. If not needed, remove it to clean up the code. If needed, uncomment it.
• Reason this comment was not posted:
  Confidence changes required: 50%
  In AddConnectionTabContent.tsx, the refetch function call is commented out in the onSuccessCallback. If it's not needed, it should be removed to clean up the code. If it is needed, it should be uncommented.

5. packages/ui/domain-components/ConnectionCard.tsx:56

• Draft comment:
  Ensure onReconnect is only available for 'reconnectable' connections as suggested by the comment. This will ensure the UI behaves as expected.
• Reason this comment was not posted:
  Confidence changes required: 50%
  In ConnectionCard.tsx, the onReconnect function is used in a DropdownMenuItem, but there's a comment suggesting it should only be for 'reconnectable' connections. This should be addressed to ensure the UI behaves as expected.

Workflow ID: wflow_mLokLlLYzbfpuUfB

---

You can customize Ellipsis with 👍 / 👎 feedback, review rules, user-specific overrides, quiet mode, and more.

[ellipsis-dev]

👍 Looks good to me! Reviewed everything up to 592742d in 1 minute and 52 seconds

More details

• Looked at 824 lines of code in 22 files
• Skipped 0 files when reviewing.
• Skipped posting 6 drafted comments based on config settings.

1. packages/ui/domain-components/ConnectionCard.tsx:38

• Draft comment:
  Consider using a callback or promise resolution to stop the spinner instead of a fixed timeout. This will ensure the spinner stops exactly when the operation completes, improving user experience.
• Reason this comment was not posted:
  Decided after close inspection that this draft comment was likely wrong and/or not actionable:
  The code explicitly acknowledges the timeout approach with "ok as upon complete it will refresh" comment. This suggests the team is aware of the tradeoff and chose this approach deliberately. The page will refresh when the operation completes anyway, so precise spinner timing isn't critical. The timeout acts as a fallback in case the refresh doesn't happen.
  The comment does point out a legitimate potential improvement in UX. There could be cases where the operation completes much faster than 60s and users would see an unnecessary spinner.
  However, given the explicit comment and the fact that the page refreshes on completion anyway, this is more of an optimization than a critical issue. The current solution, while not perfect, is functional and deliberately chosen.
  Delete the comment. The current implementation is an intentional choice with acceptable tradeoffs, and the suggestion, while valid, isn't critical enough to warrant a comment.

2. packages/ui/domain-components/ConnectionCard.tsx:47

• Draft comment:
  Consider using a callback or promise resolution to stop the spinner instead of a fixed timeout. This will ensure the spinner stops exactly when the operation completes, improving user experience.
• Reason this comment was not posted:
  Marked as duplicate.

3. packages/ui/domain-components/ConnectionCard.tsx:76

• Draft comment:
  Consider refactoring the isOAuthConnector logic into a utility function to avoid code duplication and potential inconsistencies across the codebase.
• Reason this comment was not posted:
  Confidence changes required: 50%
  The ConnectionCard component uses the isOAuthConnector check to conditionally render the "Reconnect" option. This logic is repeated in multiple places, such as in ConnectionCard.tsx and ConnectionsTabContent.tsx. It would be beneficial to refactor this logic into a utility function to avoid code duplication and potential inconsistencies.

4. packages/engine-frontend/components/ConnectionsTabContent.tsx:65

• Draft comment:
  Consider refactoring the isOAuthConnector logic into a utility function to avoid code duplication and potential inconsistencies across the codebase.
• Reason this comment was not posted:
  Confidence changes required: 50%
  The ConnectionCard component uses the isOAuthConnector check to conditionally render the "Reconnect" option. This logic is repeated in multiple places, such as in ConnectionCard.tsx and ConnectionsTabContent.tsx. It would be beneficial to refactor this logic into a utility function to avoid code duplication and potential inconsistencies.

5. packages/engine-frontend/components/ConnectionsTabContent.tsx:70

• Draft comment:
  Consider refactoring the isOAuthConnector logic into a utility function to avoid code duplication and potential inconsistencies across the codebase.
• Reason this comment was not posted:
  Confidence changes required: 50%
  The ConnectionCard component uses the isOAuthConnector check to conditionally render the "Reconnect" option. This logic is repeated in multiple places, such as in ConnectionCard.tsx and ConnectionsTabContent.tsx. It would be beneficial to refactor this logic into a utility function to avoid code duplication and potential inconsistencies.

6. packages/engine-frontend/hocs/WithConnectorConnect.tsx:136

• Draft comment:
  Consider refactoring the isOAuthConnector logic into a utility function to avoid code duplication and potential inconsistencies across the codebase.
• Reason this comment was not posted:
  Confidence changes required: 50%
  The ConnectionCard component uses the isOAuthConnector check to conditionally render the "Reconnect" option. This logic is repeated in multiple places, such as in ConnectionCard.tsx and ConnectionsTabContent.tsx. It would be beneficial to refactor this logic into a utility function to avoid code duplication and potential inconsistencies.

Workflow ID: wflow_FOlrTIHi5bO6hydL

---

You can customize Ellipsis with 👍 / 👎 feedback, review rules, user-specific overrides, quiet mode, and more.

[ellipsis-dev]

❌ Changes requested. Incremental review on 922c59f in 52 seconds

More details

• Looked at 101 lines of code in 2 files
• Skipped 0 files when reviewing.
• Skipped posting 0 drafted comments based on config settings.

Workflow ID: wflow_7FB5rvMMoUiwOjfy

---

Want Ellipsis to fix these issues? Tag @ellipsis-dev in a comment. You can customize Ellipsis with 👍 / 👎 feedback, review rules, user-specific overrides, quiet mode, and more.

[ellipsis-dev]

The timeout value here is 6000000 ms (100 minutes), which seems excessively high and likely a typo. Consider changing it to 60000 ms (1 minute) for consistency with handleReconnect.

[ellipsis-dev]

👍 Looks good to me! Incremental review on 0afc98a in 39 seconds

More details

• Looked at 63 lines of code in 2 files
• Skipped 0 files when reviewing.
• Skipped posting 1 drafted comments based on config settings.

1. packages/ui/domain-components/ConnectionCard.tsx:43

• Draft comment:
  The timeout duration for setting isProcessing to false is inconsistent between the reconnect and delete actions. Consider using a consistent timeout duration for both actions to ensure uniform user experience.
• Reason this comment was not posted:
  Comment was on unchanged code.

Workflow ID: wflow_u4ivAq3do2DPVoQf

---

You can customize Ellipsis with 👍 / 👎 feedback, review rules, user-specific overrides, quiet mode, and more.