Conversation
Co-Authored-By: yujonglee <yujonglee.dev@gmail.com>
❌ Deploy Preview for hyprnote-storybook failed.
|
❌ Deploy Preview for hyprnote failed.
|
🤖 Devin AI EngineerI'll be helping with this pull request! Here's what you should know: ✅ I will automatically:
Note: I can only respond to comments from users who have write access to this repository. ⚙️ Control Options:
|
| let mut url = self.api_base.clone(); | ||
| url.set_path(&format!("/sync/{}/variant/{}", name, variant)); | ||
|
|
||
| let response = self.client.delete(url).json(&req).send().await?; | ||
| check_response(response).await?; |
There was a problem hiding this comment.
🔴 delete_variant sends required parameters as JSON body on DELETE instead of query parameters
The delete_variant method sends provider_config_key and connection_id as a JSON body (.json(&req)) on a DELETE request. The Nango API expects these as query parameters for the DELETE /sync/{name}/variant/{variant} endpoint, meaning the server will not receive the required parameters and the call will fail.
Root Cause and Evidence from Existing Patterns
The existing delete_connection method in connection.rs:223-239 demonstrates the correct pattern for DELETE endpoints — it passes parameters as query params via append_query:
pub async fn delete_connection(...) {
url.set_path(&format!("/connections/{}", connection_id));
append_query(&mut url, "provider_config_key", &provider_config_key.to_string());
let response = self.client.delete(url).send().await?;
...
}Similarly, delete_integration in integration.rs:155-165 sends no body at all on DELETE.
In contrast, delete_variant at crates/nango/src/sync.rs:347 uses .json(&req) on a DELETE request:
let response = self.client.delete(url).json(&req).send().await?;This sends {"provider_config_key":"...","connection_id":"..."} in the request body, but the Nango API expects ?provider_config_key=...&connection_id=... as query parameters. The API will reject the request or behave unexpectedly because the required query parameters are missing.
Impact: Every call to delete_variant will fail because the required provider_config_key and connection_id parameters are not sent where the API expects them.
| let mut url = self.api_base.clone(); | |
| url.set_path(&format!("/sync/{}/variant/{}", name, variant)); | |
| let response = self.client.delete(url).json(&req).send().await?; | |
| check_response(response).await?; | |
| let mut url = self.api_base.clone(); | |
| url.set_path(&format!("/sync/{}/variant/{}", name, variant)); | |
| append_query(&mut url, "provider_config_key", &req.provider_config_key); | |
| append_query(&mut url, "connection_id", &req.connection_id); | |
| let response = self.client.delete(url).send().await?; |
Was this helpful? React with 👍 or 👎 to provide feedback.
add sync module to nango crate
Summary
Adds
sync.rstocrates/nangoimplementing typed Rust bindings for all 10 Nango Sync API endpoints:GET /records→get_recordsPATCH /records/prune→prune_recordsPOST /sync/trigger→trigger_syncPOST /sync/start→start_syncPOST /sync/pause→pause_syncGET /sync/status→sync_statusPUT /sync/update-connection-frequency→update_connection_frequencyGET /environment-variables→get_environment_variablesPOST /sync/{name}/variant/{variant}→create_variantDELETE /sync/{name}/variant/{variant}→delete_variantFollows existing patterns from
connection.rsandintegration.rs— usescommon_derives!,append_query,check_response/parse_response,#[serde(skip)]on param structs, etc.Review & Testing Checklist for Human
SyncStatusfield casing:finishedAt,nextScheduledSyncAt,latestResultuse camelCase#[serde(rename)]. If the actual Nango API returns snake_case for these, deserialization will silently produceNone. Verify against a real API response.prune_recordsrequest shape: Docs page was truncated. Current impl assumesConnection-Id/Provider-Config-Keyheaders +modelquery param (mirroringGET /records). Confirm this matches the actual API.idsquery param format inget_records: Currently appends multipleids=Xpairs. Verify the Nango API doesn't expectids[]=Xor comma-separated format.NangoRecordMetadatais exported but not referenced in any method signature — confirm it's useful as a downstream helper type or remove it.Notes
SyncStatus.statusandSyncStatus.typeareString(not enums) — intentional for flexibility since Nango could add new values.GetRecordsResponse.recordsisVec<serde_json::Value>since record shapes are user-defined per sync script.#[ignore]integration tests that require live credentials.Requested by: @yujonglee
Link to Devin run