-
Notifications
You must be signed in to change notification settings - Fork 20
fix(connectors): serve catalog from a build-time manifest, stop cold-scan 503s #1013
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Merged
Merged
Changes from all commits
Commits
Show all changes
3 commits
Select commit
Hold shift + click to select a range
File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
38 changes: 38 additions & 0 deletions
38
packages/server/scripts/build-connector-catalog-manifest.ts
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,38 @@ | ||
| /** | ||
| * Build-time generator for the connector catalog manifest. Compiles every | ||
| * bundled connector once and writes dist/connectors/.catalog-manifest.json so | ||
| * the server serves the catalog without recompiling on demand (see | ||
| * CATALOG_MANIFEST_FILENAME in connector-catalog.ts for the why). | ||
| * | ||
| * Runs after build-server-bundle.mjs (which copies the sources into | ||
| * dist/connectors). Executed under `bun` so it can import the TS catalog code. | ||
| */ | ||
| import { existsSync } from 'node:fs'; | ||
| import { writeFile } from 'node:fs/promises'; | ||
| import { dirname, join } from 'node:path'; | ||
| import { fileURLToPath } from 'node:url'; | ||
| import { | ||
| CATALOG_MANIFEST_FILENAME, | ||
| generateCatalogManifest, | ||
| } from '../src/utils/connector-catalog'; | ||
|
|
||
| const here = dirname(fileURLToPath(import.meta.url)); | ||
| const connectorsDir = join(here, '..', 'dist', 'connectors'); | ||
|
|
||
| if (!existsSync(connectorsDir)) { | ||
| console.warn( | ||
| `[catalog-manifest] ${connectorsDir} missing; skipping (run build:server first).` | ||
| ); | ||
| process.exit(0); | ||
| } | ||
|
Comment on lines
+22
to
+27
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Fail the build when Line 30 exits with code 0, which can hide a broken bundle step and ship without a manifest, reintroducing timeout-prone runtime scans. Suggested change if (!existsSync(connectorsDir)) {
- console.warn(
- `[catalog-manifest] ${connectorsDir} missing; skipping (run build:server first).`
- );
- process.exit(0);
+ throw new Error(
+ `[catalog-manifest] ${connectorsDir} missing; expected after build-server-bundle.mjs`
+ );
}🤖 Prompt for AI Agents |
||
|
|
||
| const start = Date.now(); | ||
| const manifest = await generateCatalogManifest(connectorsDir); | ||
| const manifestPath = join(connectorsDir, CATALOG_MANIFEST_FILENAME); | ||
| await writeFile(manifestPath, `${JSON.stringify(manifest, null, 2)}\n`, 'utf-8'); | ||
|
|
||
| const total = Object.keys(manifest.entries).length; | ||
| const connectors = Object.values(manifest.entries).filter(Boolean).length; | ||
| console.log( | ||
| `\n=== connector catalog manifest: ${connectors} connectors / ${total} files -> ${manifestPath} (${Date.now() - start}ms)` | ||
| ); | ||
58 changes: 58 additions & 0 deletions
58
packages/server/src/__tests__/integration/connectors/sync-run-orphan-feed.test.ts
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,58 @@ | ||
| /** | ||
| * createSyncRun orphan-feed handling (#1012). | ||
| * | ||
| * A feed whose connector resolves to a definition + version row that has no | ||
| * compiled code and no bundled source file (the prod `chrome.tabs` state) can | ||
| * never run. Pre-fix, createSyncRun threw on every CheckDueFeeds tick, storming | ||
| * the logs with a per-poll error and never making progress. It must instead | ||
| * soft-delete the orphan feed (mirroring the no-definition path) so it stops | ||
| * appearing in CheckDueFeeds. | ||
| */ | ||
| import { beforeEach, describe, expect, it } from 'vitest'; | ||
| import type { Env } from '../../../index'; | ||
| import { createSyncRun } from '../../../utils/queue-helpers'; | ||
| import { cleanupTestDatabase, getTestDb } from '../../setup/test-db'; | ||
| import { | ||
| createTestConnection, | ||
| createTestConnectorDefinition, | ||
| createTestOrganization, | ||
| } from '../../setup/test-fixtures'; | ||
|
|
||
| describe('createSyncRun orphan-feed handling (#1012)', () => { | ||
| beforeEach(async () => { | ||
| await cleanupTestDatabase(); | ||
| }); | ||
|
|
||
| it('soft-deletes a feed whose connector has no compiled code and no bundled source instead of throwing', async () => { | ||
| const sql = getTestDb(); | ||
| const org = await createTestOrganization(); | ||
|
|
||
| // Definition + version exist (so this is NOT the no-definition orphan path)… | ||
| await createTestConnectorDefinition({ | ||
| key: 'orphan.no_code', | ||
| name: 'Orphan No Code', | ||
| organization_id: org.id, | ||
| }); | ||
| // …but the version carries no runnable code, and the key is not a bundled | ||
| // connector — exactly the prod `chrome.tabs` state that threw every poll. | ||
| await sql`UPDATE connector_versions SET compiled_code = NULL WHERE connector_key = 'orphan.no_code'`; | ||
|
|
||
| const conn = await createTestConnection({ | ||
| organization_id: org.id, | ||
| connector_key: 'orphan.no_code', | ||
| }); | ||
| const [feed] = await sql`SELECT id FROM feeds WHERE connection_id = ${conn.id}`; | ||
| const feedId = Number((feed as { id: number }).id); | ||
|
|
||
| // Pre-fix: threw "has no compiled code and no bundled source file". | ||
| const runId = await createSyncRun(feedId, {} as Env, sql); | ||
| expect(runId).toBeNull(); | ||
|
|
||
| const [after] = await sql`SELECT deleted_at FROM feeds WHERE id = ${feedId}`; | ||
| expect((after as { deleted_at: Date | null }).deleted_at).not.toBeNull(); | ||
|
|
||
| // No run row was created for the orphan feed. | ||
| const runs = await sql`SELECT id FROM runs WHERE feed_id = ${feedId}`; | ||
| expect(runs.length).toBe(0); | ||
| }); | ||
| }); |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🛠️ Refactor suggestion | 🟠 Major | ⚡ Quick win
Move this TypeScript script under
packages/server/src.This new
.tssource file is inpackages/server/scripts, which violates the repo TS file placement rule. Move it underpackages/server/src/...(and updatebuild:serveraccordingly).As per coding guidelines,
**/*.ts: Place TypeScript source code inpackages/*/srcdirectory and tests in__tests__directory within each workspace.🤖 Prompt for AI Agents