mirror of
https://github.com/simstudioai/sim.git
synced 2026-09-24 15:45:35 +08:00
fix(trigger): fix polling trigger config defaults, row count, clock-skew, and stale config clearing (#4101)
* fix(trigger): fix polling trigger config defaults, row count, clock-skew, and stale config clearing * fix(deploy): track first-pass fills to prevent stale baseConfig bypassing required-field validation Use a dedicated `filledSubBlockIds` Set populated during the first pass so the second-pass skip guard is based solely on live `getConfigValue` results, not on stale entries spread from `baseConfig` (`triggerConfig`). * fix(trigger): prevent calendar cursor regression when all events are filtered client-side
This commit is contained in:
@@ -66,7 +66,6 @@ export function useTriggerConfigAggregation(
|
||||
let valueToUse = fieldValue
|
||||
if (
|
||||
(fieldValue === null || fieldValue === undefined || fieldValue === '') &&
|
||||
subBlock.required &&
|
||||
subBlock.defaultValue !== undefined
|
||||
) {
|
||||
valueToUse = subBlock.defaultValue
|
||||
|
||||
@@ -13,6 +13,7 @@ import {
|
||||
} from '@/lib/webhooks/provider-subscriptions'
|
||||
import { getProviderHandler } from '@/lib/webhooks/providers'
|
||||
import { syncWebhooksForCredentialSet } from '@/lib/webhooks/utils.server'
|
||||
import { buildCanonicalIndex } from '@/lib/workflows/subblocks/visibility'
|
||||
import { getBlock } from '@/blocks'
|
||||
import type { SubBlockConfig } from '@/blocks/types'
|
||||
import type { BlockState } from '@/stores/workflows/workflow/types'
|
||||
@@ -150,7 +151,6 @@ function getConfigValue(block: BlockState, subBlock: SubBlockConfig): unknown {
|
||||
|
||||
if (
|
||||
(fieldValue === null || fieldValue === undefined || fieldValue === '') &&
|
||||
Boolean(subBlock.required) &&
|
||||
subBlock.defaultValue !== undefined
|
||||
) {
|
||||
return subBlock.defaultValue
|
||||
@@ -182,20 +182,40 @@ function buildProviderConfig(
|
||||
Object.entries(block.subBlocks || {}).map(([key, value]) => [key, { value: value.value }])
|
||||
)
|
||||
|
||||
triggerDef.subBlocks
|
||||
.filter(
|
||||
(subBlock) =>
|
||||
(subBlock.mode === 'trigger' || subBlock.mode === 'trigger-advanced') &&
|
||||
!SYSTEM_SUBBLOCK_IDS.includes(subBlock.id)
|
||||
)
|
||||
.forEach((subBlock) => {
|
||||
const valueToUse = getConfigValue(block, subBlock)
|
||||
if (valueToUse !== null && valueToUse !== undefined && valueToUse !== '') {
|
||||
providerConfig[subBlock.id] = valueToUse
|
||||
} else if (isFieldRequired(subBlock, subBlockValues)) {
|
||||
missingFields.push(subBlock.title || subBlock.id)
|
||||
}
|
||||
})
|
||||
const canonicalIndex = buildCanonicalIndex(triggerDef.subBlocks)
|
||||
const satisfiedCanonicalIds = new Set<string>()
|
||||
const filledSubBlockIds = new Set<string>()
|
||||
|
||||
const relevantSubBlocks = triggerDef.subBlocks.filter(
|
||||
(subBlock) =>
|
||||
(subBlock.mode === 'trigger' || subBlock.mode === 'trigger-advanced') &&
|
||||
!SYSTEM_SUBBLOCK_IDS.includes(subBlock.id)
|
||||
)
|
||||
|
||||
// First pass: populate providerConfig, clear stale baseConfig entries, and track which
|
||||
// subblocks and canonical groups have a value.
|
||||
for (const subBlock of relevantSubBlocks) {
|
||||
const valueToUse = getConfigValue(block, subBlock)
|
||||
if (valueToUse !== null && valueToUse !== undefined && valueToUse !== '') {
|
||||
providerConfig[subBlock.id] = valueToUse
|
||||
filledSubBlockIds.add(subBlock.id)
|
||||
const canonicalId = canonicalIndex.canonicalIdBySubBlockId[subBlock.id]
|
||||
if (canonicalId) satisfiedCanonicalIds.add(canonicalId)
|
||||
} else {
|
||||
delete providerConfig[subBlock.id]
|
||||
}
|
||||
}
|
||||
|
||||
// Second pass: validate required fields. Skip subblocks that are filled or whose canonical
|
||||
// group is satisfied by another member.
|
||||
for (const subBlock of relevantSubBlocks) {
|
||||
if (filledSubBlockIds.has(subBlock.id)) continue
|
||||
const canonicalId = canonicalIndex.canonicalIdBySubBlockId[subBlock.id]
|
||||
if (canonicalId && satisfiedCanonicalIds.has(canonicalId)) continue
|
||||
if (isFieldRequired(subBlock, subBlockValues)) {
|
||||
missingFields.push(subBlock.title || subBlock.id)
|
||||
}
|
||||
}
|
||||
|
||||
const credentialConfig = triggerDef.subBlocks.find(
|
||||
(subBlock) => subBlock.id === 'triggerCredentials'
|
||||
|
||||
@@ -106,7 +106,7 @@ export const googleCalendarPollingHandler: PollingProviderHandler = {
|
||||
if (!config.lastCheckedTimestamp) {
|
||||
await updateWebhookProviderConfig(
|
||||
webhookId,
|
||||
{ lastCheckedTimestamp: new Date().toISOString() },
|
||||
{ lastCheckedTimestamp: new Date(Date.now() - 30_000).toISOString() },
|
||||
logger
|
||||
)
|
||||
await markWebhookSuccess(webhookId, logger)
|
||||
@@ -135,11 +135,19 @@ export const googleCalendarPollingHandler: PollingProviderHandler = {
|
||||
logger
|
||||
)
|
||||
|
||||
// Advance cursor to latestUpdated - 5s for clock-skew overlap, but never regress
|
||||
// below the previous cursor — this prevents an infinite re-fetch loop when all
|
||||
// returned events are filtered client-side and latestUpdated is within 5s of the cursor.
|
||||
const newTimestamp =
|
||||
failedCount > 0
|
||||
? config.lastCheckedTimestamp
|
||||
: latestUpdated
|
||||
? new Date(new Date(latestUpdated).getTime() + 1).toISOString()
|
||||
? new Date(
|
||||
Math.max(
|
||||
new Date(latestUpdated).getTime() - 5000,
|
||||
new Date(config.lastCheckedTimestamp).getTime()
|
||||
)
|
||||
).toISOString()
|
||||
: config.lastCheckedTimestamp
|
||||
await updateWebhookProviderConfig(webhookId, { lastCheckedTimestamp: newTimestamp }, logger)
|
||||
|
||||
@@ -182,7 +190,6 @@ async function fetchChangedEvents(
|
||||
updatedMin: config.lastCheckedTimestamp!,
|
||||
singleEvents: 'true',
|
||||
showDeleted: 'true',
|
||||
orderBy: 'updated',
|
||||
maxResults: String(Math.min(maxEvents, 250)),
|
||||
})
|
||||
|
||||
|
||||
@@ -145,7 +145,11 @@ export const googleDrivePollingHandler: PollingProviderHandler = {
|
||||
logger
|
||||
)
|
||||
|
||||
// Update state: new pageToken and rolling knownFileIds
|
||||
// Update state: new pageToken and rolling knownFileIds.
|
||||
// Newest IDs are placed first so that when the set exceeds MAX_KNOWN_FILE_IDS,
|
||||
// the oldest (least recently seen) IDs are evicted. Recent files are more
|
||||
// likely to be modified again, so keeping them prevents misclassifying a
|
||||
// repeat modification as a "created" event.
|
||||
const existingKnownIds = config.knownFileIds || []
|
||||
const mergedKnownIds = [...new Set([...newKnownFileIds, ...existingKnownIds])].slice(
|
||||
0,
|
||||
@@ -271,9 +275,14 @@ async function fetchChanges(
|
||||
}
|
||||
|
||||
const slicingOccurs = allChanges.length > maxFiles
|
||||
// Drive API guarantees exactly one of nextPageToken or newStartPageToken per response.
|
||||
// Slicing case: prefer lastNextPageToken (mid-list resume); fall back to newStartPageToken
|
||||
// (guaranteed on final page when hasMore was false). Non-slicing case: prefer newStartPageToken
|
||||
// (guaranteed when loop exhausted all pages); fall back to lastNextPageToken (when loop exited
|
||||
// early due to MAX_PAGES with hasMore still true).
|
||||
const resumeToken = slicingOccurs
|
||||
? (lastNextPageToken ?? config.pageToken!)
|
||||
: (newStartPageToken ?? lastNextPageToken ?? config.pageToken!)
|
||||
? (lastNextPageToken ?? newStartPageToken!)
|
||||
: (newStartPageToken ?? lastNextPageToken!)
|
||||
|
||||
return { changes: allChanges.slice(0, maxFiles), newStartPageToken: resumeToken }
|
||||
}
|
||||
|
||||
@@ -18,7 +18,6 @@ interface GoogleSheetsWebhookConfig {
|
||||
manualSpreadsheetId?: string
|
||||
sheetName?: string
|
||||
manualSheetName?: string
|
||||
includeHeaders: boolean
|
||||
valueRenderOption?: ValueRenderOption
|
||||
dateTimeRenderOption?: DateTimeRenderOption
|
||||
lastKnownRowCount?: number
|
||||
@@ -147,19 +146,15 @@ export const googleSheetsPollingHandler: PollingProviderHandler = {
|
||||
const valueRender = config.valueRenderOption || 'FORMATTED_VALUE'
|
||||
const dateTimeRender = config.dateTimeRenderOption || 'SERIAL_NUMBER'
|
||||
|
||||
// Fetch headers (row 1) if includeHeaders is enabled
|
||||
let headers: string[] = []
|
||||
if (config.includeHeaders !== false) {
|
||||
headers = await fetchHeaderRow(
|
||||
accessToken,
|
||||
spreadsheetId,
|
||||
sheetName,
|
||||
valueRender,
|
||||
dateTimeRender,
|
||||
requestId,
|
||||
logger
|
||||
)
|
||||
}
|
||||
const headers = await fetchHeaderRow(
|
||||
accessToken,
|
||||
spreadsheetId,
|
||||
sheetName,
|
||||
valueRender,
|
||||
dateTimeRender,
|
||||
requestId,
|
||||
logger
|
||||
)
|
||||
|
||||
// Fetch new rows — startRow/endRow are already 1-indexed sheet row numbers
|
||||
// because lastKnownRowCount includes the header row
|
||||
@@ -269,7 +264,12 @@ async function getDataRowCount(
|
||||
logger: ReturnType<typeof import('@sim/logger').createLogger>
|
||||
): Promise<number> {
|
||||
const encodedSheet = encodeURIComponent(sheetName)
|
||||
const url = `https://sheets.googleapis.com/v4/spreadsheets/${spreadsheetId}/values/${encodedSheet}!A:A?majorDimension=COLUMNS&fields=values`
|
||||
// Fetch all rows across columns A–Z with majorDimension=ROWS so the API
|
||||
// returns one entry per row that has ANY non-empty cell. Rows where column A
|
||||
// is empty but other columns have data are included, whereas the previous
|
||||
// column-A-only approach silently missed them. The returned array length
|
||||
// equals the 1-indexed row number of the last row with data.
|
||||
const url = `https://sheets.googleapis.com/v4/spreadsheets/${spreadsheetId}/values/${encodedSheet}!A:Z?majorDimension=ROWS&fields=values`
|
||||
|
||||
const response = await fetch(url, {
|
||||
headers: { Authorization: `Bearer ${accessToken}` },
|
||||
@@ -291,9 +291,11 @@ async function getDataRowCount(
|
||||
}
|
||||
|
||||
const data = await response.json()
|
||||
// values is [[cell1, cell2, ...]] when majorDimension=COLUMNS
|
||||
const columnValues = data.values?.[0] as string[] | undefined
|
||||
return columnValues?.length ?? 0
|
||||
// values is [[row1col1, row1col2, ...], [row2col1, ...], ...] when majorDimension=ROWS.
|
||||
// The Sheets API omits trailing empty rows, so the array length is the last
|
||||
// non-empty row index (1-indexed), which is exactly what we need.
|
||||
const rows = data.values as string[][] | undefined
|
||||
return rows?.length ?? 0
|
||||
}
|
||||
|
||||
async function fetchHeaderRow(
|
||||
@@ -399,15 +401,12 @@ async function processRows(
|
||||
'google-sheets',
|
||||
`${webhookData.id}:${spreadsheetId}:${sheetName}:row${rowNumber}`,
|
||||
async () => {
|
||||
// Map row values to headers
|
||||
let mappedRow: Record<string, string> | null = null
|
||||
if (headers.length > 0 && config.includeHeaders !== false) {
|
||||
if (headers.length > 0) {
|
||||
mappedRow = {}
|
||||
for (let j = 0; j < headers.length; j++) {
|
||||
const header = headers[j] || `Column ${j + 1}`
|
||||
mappedRow[header] = row[j] ?? ''
|
||||
mappedRow[headers[j] || `Column ${j + 1}`] = row[j] ?? ''
|
||||
}
|
||||
// Include any extra columns beyond headers
|
||||
for (let j = headers.length; j < row.length; j++) {
|
||||
mappedRow[`Column ${j + 1}`] = row[j] ?? ''
|
||||
}
|
||||
|
||||
@@ -11,6 +11,7 @@ export const SYSTEM_SUBBLOCK_IDS: string[] = [
|
||||
'samplePayload', // Example payload display
|
||||
'setupScript', // Setup script code (e.g., Apps Script)
|
||||
'scheduleInfo', // Schedule status display (next run, last run)
|
||||
'triggerSave', // UI-only save button — stores no config data
|
||||
]
|
||||
|
||||
/**
|
||||
|
||||
@@ -69,16 +69,6 @@ export const googleSheetsPollingTrigger: TriggerConfig = {
|
||||
mode: 'trigger-advanced',
|
||||
canonicalParamId: 'sheetName',
|
||||
},
|
||||
{
|
||||
id: 'includeHeaders',
|
||||
title: 'Map Row Values to Headers',
|
||||
type: 'switch',
|
||||
defaultValue: true,
|
||||
description:
|
||||
'When enabled, each row is returned as a key-value object mapped to column headers from row 1.',
|
||||
required: false,
|
||||
mode: 'trigger',
|
||||
},
|
||||
{
|
||||
id: 'valueRenderOption',
|
||||
title: 'Value Render',
|
||||
@@ -139,7 +129,7 @@ export const googleSheetsPollingTrigger: TriggerConfig = {
|
||||
outputs: {
|
||||
row: {
|
||||
type: 'json',
|
||||
description: 'Row data mapped to column headers (when header mapping is enabled)',
|
||||
description: 'Row data mapped to column headers from row 1',
|
||||
},
|
||||
rawRow: {
|
||||
type: 'json',
|
||||
|
||||
Reference in New Issue
Block a user