feat: implement shared ConnectionProvider from @relayfile/sdk#2
feat: implement shared ConnectionProvider from @relayfile/sdk#2khaliqgant merged 9 commits intomainfrom
Conversation
![devin-ai-integration[bot]](https://avatars.githubusercontent.com/in/811515?s=60&v=4){kind=small}
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
khaliqgant
force-pushed
the
feat/shared-connection-provider
branch
from
b579b2a to
ed0e3cb
Compare
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
| objectType: String(record.objectType ?? record.object_type), | ||
| objectId: String(record.objectId ?? record.object_id), | ||
| eventType: String(record.eventType ?? record.event_type), | ||
| payload: | ||
| isObject(record.payload) || isObject(record.data) | ||
| ? (record.payload ?? record.data) | ||
| payload: isObject(record.payload) | ||
| ? record.payload | ||
| : isObject(record.data) | ||
| ? record.data | ||
| : record, | ||
| raw: rawInput, | ||
| metadata: readStringMap(record.metadata), | ||
| } as NormalizedWebhook; | ||
| }; |
There was a problem hiding this comment.
🔴 Pipedream webhook normalization missing event field required by SdkNormalizedWebhook
All four return paths in normalizePipedreamWebhook() are missing the event field. PipedreamNormalizedWebhook extends SdkNormalizedWebhook (packages/pipedream/src/types.ts:60-61), which includes an event field — every other provider (nango, composio, n8n, clerk, supabase) consistently sets event: eventType in their webhook normalization. For example, packages/nango/src/webhook.ts adds event: metadata.eventType, packages/composio/src/webhook.ts adds event: eventType, packages/n8n/src/webhook.ts adds event: eventType, etc. The pipedream webhook was updated to add raw: rawInput (matching the new SDK contract) but event was never added. If event is required in SdkNormalizedWebhook, the TypeScript build will fail. If event is optional, consumers of the webhook object will get undefined for .event only from the pipedream provider, while all other providers correctly populate it.
(Refers to lines 9-25)
Was this helpful? React with 👍 or 👎 to provide feedback.
| objectId: accountId, | ||
| eventType: "connected", | ||
| payload: record, | ||
| raw: rawInput, | ||
| metadata: buildMetadata(record), | ||
| } as NormalizedWebhook; | ||
| }; |
There was a problem hiding this comment.
🔴 Pipedream CONNECTION_SUCCESS webhook path missing event field
The CONNECTION_SUCCESS return path in normalizePipedreamWebhook() is missing the event field required by SdkNormalizedWebhook. It sets eventType: "connected" but does not set event. All other providers set both fields consistently (e.g., event: eventType).
(Refers to lines 37-46)
Was this helpful? React with 👍 or 👎 to provide feedback.
| objectId, | ||
| eventType: "connection_error", | ||
| payload: record, | ||
| raw: rawInput, | ||
| metadata: buildMetadata(record), | ||
| } as NormalizedWebhook; | ||
| }; |
There was a problem hiding this comment.
🔴 Pipedream CONNECTION_ERROR webhook path missing event field
The CONNECTION_ERROR return path in normalizePipedreamWebhook() is missing the event field required by SdkNormalizedWebhook. It sets eventType: "connection_error" but does not set event.
(Refers to lines 51-60)
Was this helpful? React with 👍 or 👎 to provide feedback.
| objectId, | ||
| eventType, | ||
| payload: record, | ||
| raw: rawInput, | ||
| metadata: readStringMap(record.metadata), | ||
| } as NormalizedWebhook; | ||
| }; |
There was a problem hiding this comment.
🔴 Pipedream generic/fallback webhook path missing event field
The generic/fallback return path in normalizePipedreamWebhook() (lines 87-96) is missing the event field required by SdkNormalizedWebhook. It sets eventType but does not set event. This is the fourth and final return path that is missing the field.
(Refers to lines 87-96)
Was this helpful? React with 👍 or 👎 to provide feedback.
khaliqgant
force-pushed
the
feat/shared-connection-provider
branch
from
20ea7de to
76030e1
Compare
packages/nango/src/nango-provider.ts
Outdated
|
|
||
| return getNangoConnection(this.config, connectionId, normalizedOptions); | ||
| const connection = await getNangoConnection(this.config, connectionId, normalizedOptions); | ||
| return options === undefined ? (connection?.raw ?? {}) : connection; |
There was a problem hiding this comment.
🔴 getConnection() no-args overload declares non-nullable return type but can return null
The new getConnection(connectionId: string) overload at line 71 declares return type Promise<Record<string, unknown>> (non-nullable). However, the implementation at line 86-87 returns connection?.raw ?? null. When getNangoConnection returns null (connection not found), connection?.raw is undefined, so undefined ?? null evaluates to null. Callers using this overload trust the type and will access properties on the result without a null check, causing runtime crashes.
The previous implementation before this PR declared Promise<NangoConnection | null>, correctly reflecting the nullable case. The refactoring into overloads lost this nullability.
Was this helpful? React with 👍 or 👎 to provide feedback.
| const connections = await nango.listConnections(); | ||
| console.log(`Found ${connections.length} connection(s)`); | ||
| for (const conn of connections) { | ||
| console.log(` - ${conn.connectionId} (provider: ${conn.providerConfigKey})`); | ||
| } | ||
| } catch (err) { | ||
| console.log("List connections failed (expected):", (err as Error).message); |
There was a problem hiding this comment.
🟡 listConnections no-args overload accesses .raw which may have inconsistent property naming
The no-args listConnections() overload at packages/nango/src/nango-provider.ts:130 maps connections to connection.raw, returning NangoConnectionRecord objects. The example at examples/05-health-and-connections/index.ts:70 was updated to use conn.connection_id and conn.provider_config_key (snake_case). However, NangoConnectionRecord (packages/nango/src/types.ts:86-88) defines both connection_id and connectionId as optional fields — the actual value present depends on the Nango API response format. If the API returns camelCase (connectionId), then conn.connection_id will be undefined, silently printing undefined in the example output. The previous code used the normalized NangoConnection.connectionId which was always populated.
Was this helpful? React with 👍 or 👎 to provide feedback.
khaliqgant
force-pushed
the
feat/shared-connection-provider
branch
from
76030e1 to
95c86b3
Compare
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
khaliqgant
force-pushed
the
feat/shared-connection-provider
branch
from
a85f2b2 to
48938db
Compare
1 participant
All 6 providers implement the shared interface. Removes duplicate types.