Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly
Appearance settings

Return 409 error when canceling completed crawl jobs #2215

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.

Sign up for GitHub

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

Open
wants to merge 8 commits into
base: main
Choose a base branch
Loading
from
+105 −4
Open

Return 409 error when canceling completed crawl jobs #2215

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
8 commits
Select commit Hold shift + click to select a range
68677d8
Return 409 error when canceling completed crawl/batch scrape jobs
devin-ai-integration[bot] Sep 26, 2025
974f803
Fix code duplication: export isCrawlFinished from crawl-redis and rem…
devin-ai-integration[bot] Sep 26, 2025
008287e
Update apps/api/src/controllers/v0/crawl-cancel.ts
micahstairs Sep 26, 2025
f590f3f
Address PR feedback: fix v1 cancel URL, improve test reliability, and…
devin-ai-integration[bot] Sep 26, 2025
8b6a04f
Revert test file changes: remove cancel endpoint tests
devin-ai-integration[bot] Sep 26, 2025
a7aaf0e
Update error message to be more specific
devin-ai-integration[bot] Sep 26, 2025
c6f29d8
Update OpenAPI error messages to be more specific
devin-ai-integration[bot] Sep 26, 2025
91ee2bf
Update OpenAPI 409 response descriptions to be more specific
devin-ai-integration[bot] Sep 26, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
32 changes: 32 additions & 0 deletions 32 apps/api/openapi.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -378,6 +378,22 @@
}
}
},
"409": {
"description": "Job already completed",
"content": {
"application/json": {
"schema": {
"type": "object",
"properties": {
"error": {
"type": "string",
"example": "Cannot cancel job that has already completed"
}
}
}
}
}
},
"500": {
"description": "Server error",
"content": {
Expand Down Expand Up @@ -607,6 +623,22 @@
}
}
},
"409": {
"description": "Job already completed",
"content": {
"application/json": {
"schema": {
"type": "object",
"properties": {
"error": {
"type": "string",
"example": "Cannot cancel job that has already completed"
}
}
}
}
}
},
"500": {
"description": "Server error",
"content": {
Expand Down
82 changes: 82 additions & 0 deletions 82 apps/api/src/__tests__/snips/v1/batch-scrape.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -94,4 +94,86 @@ describe("Batch scrape tests", () => {
},
scrapeTimeout,
);

describe("DELETE /v1/batch/scrape/{id}", () => {
it.concurrent(
"should return 409 when trying to cancel a completed batch scrape job",
async () => {
const response = await batchScrape(
{
urls: ["https://example.com"],
},
identity,
);

expect(response.success).toBe(true);
if (response.success) {
const batchId = response.id;

const cancelResponse = await fetch(
`${process.env.TEST_API_URL}/v1/batch/scrape/${batchId}`,
{
method: "DELETE",
headers: {
Authorization: `Bearer ${identity.apiKey}`,
"Content-Type": "application/json",
},
},
);

expect(cancelResponse.status).toBe(409);
const cancelData = await cancelResponse.json();
expect(cancelData.error).toBe(
"Cannot cancel job that has already completed",
);
}
},
scrapeTimeout,
);

it.concurrent(
"should successfully cancel an in-progress batch scrape job",
async () => {
const startResponse = await fetch(
`${process.env.TEST_API_URL}/v1/batch/scrape`,
{
method: "POST",
headers: {
Authorization: `Bearer ${identity.apiKey}`,
"Content-Type": "application/json",
},
body: JSON.stringify({
urls: Array.from(
{ length: 10 },
(_, i) => `https://firecrawl.dev/page-${i}`,
),
}),
},
);

expect(startResponse.status).toBe(200);
const startData = await startResponse.json();
const batchId = startData.id;

await new Promise(resolve => setTimeout(resolve, 1000));
Copy link
Contributor

@cubic-dev-ai cubic-dev-ai bot Sep 26, 2025 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed 1s delay before cancelling in-progress job is brittle and may cause flaky test outcomes; gate on status or use a longer, safer delay.

Prompt for AI agents 
Address the following comment on apps/api/src/__tests__/snips/v1/batch-scrape.test.ts at line 158:

<comment>Fixed 1s delay before cancelling in-progress job is brittle and may cause flaky test outcomes; gate on status or use a longer, safer delay.</comment>

<file context>
@@ -94,4 +94,86 @@ describe(&quot;Batch scrape tests&quot;, () =&gt; {
+        const startData = await startResponse.json();
+        const batchId = startData.id;
+
+        await new Promise(resolve =&gt; setTimeout(resolve, 1000));
+
+        const cancelResponse = await fetch(
</file context>

✅ Addressed in f590f3f


const cancelResponse = await fetch(
`${process.env.TEST_API_URL}/v1/batch/scrape/${batchId}`,
{
method: "DELETE",
headers: {
Authorization: `Bearer ${identity.apiKey}`,
"Content-Type": "application/json",
},
},
);

expect(cancelResponse.status).toBe(200);
const cancelData = await cancelResponse.json();
expect(cancelData.success).toBe(true);
expect(cancelData.message).toContain("cancelled");
micahstairs marked this conversation as resolved.
Outdated Show resolved Hide resolved
},
scrapeTimeout,
);
});
});
72 changes: 72 additions & 0 deletions 72 apps/api/src/__tests__/snips/v1/crawl.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -386,4 +386,76 @@ describe("Crawl tests", () => {
expect(typeof response.body.id).toBe("string");
},
);

describe("POST /v1/crawl/{id}/cancel", () => {
it.concurrent(
"should return 409 when trying to cancel a completed crawl job",
async () => {
const res = await crawl(
{
url: "https://example.com",
limit: 1,
},
identity,
);

expect(res.success).toBe(true);
if (res.success) {
const crawlId = res.id;

const cancelResponse = await fetch(
`${process.env.TEST_API_URL}/v1/crawl/${crawlId}/cancel`,
Copy link
Contributor

@cubic-dev-ai cubic-dev-ai bot Sep 26, 2025 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Wrong v1 cancel URL; should be DELETE /v1/crawl/{id} (no /cancel).

Prompt for AI agents 
Address the following comment on apps/api/src/__tests__/snips/v1/crawl.test.ts at line 407:

<comment>Wrong v1 cancel URL; should be DELETE /v1/crawl/{id} (no /cancel).</comment>

<file context>
@@ -386,4 +386,76 @@ describe(&quot;Crawl tests&quot;, () =&gt; {
+          const crawlId = res.id;
+
+          const cancelResponse = await fetch(
+            `${process.env.TEST_API_URL}/v1/crawl/${crawlId}/cancel`,
+            {
+              method: &quot;DELETE&quot;,
</file context>
Suggested change
`${process.env.TEST_API_URL}/v1/crawl/${crawlId}/cancel`,
`${process.env.TEST_API_URL}/v1/crawl/${crawlId}`,

✅ Addressed in f590f3f

{
method: "DELETE",
headers: {
Authorization: `Bearer ${identity.apiKey}`,
"Content-Type": "application/json",
},
},
);

expect(cancelResponse.status).toBe(409);
const cancelData = await cancelResponse.json();
expect(cancelData.error).toBe(
"Cannot cancel job that has already completed",
);
}
},
scrapeTimeout,
);

it.concurrent(
"should successfully cancel an in-progress crawl job",
async () => {
const startResponse = await crawlStart(
{
url: "https://firecrawl.dev",
limit: 100,
},
identity,
);

expect(startResponse.statusCode).toBe(200);
const crawlId = startResponse.body.id;

await new Promise(resolve => setTimeout(resolve, 1000));

const cancelResponse = await fetch(
`${process.env.TEST_API_URL}/v1/crawl/${crawlId}/cancel`,
{
method: "DELETE",
headers: {
Authorization: `Bearer ${identity.apiKey}`,
"Content-Type": "application/json",
},
},
);

expect(cancelResponse.status).toBe(200);
const cancelData = await cancelResponse.json();
expect(cancelData.status).toBe("cancelled");
},
scrapeTimeout,
);
});
});
29 changes: 28 additions & 1 deletion 29 apps/api/src/controllers/v0/crawl-cancel.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,12 +2,28 @@ import { Request, Response } from "express";
import { authenticateUser } from "../auth";
import { RateLimiterMode } from "../../../src/types";
import { logger } from "../../../src/lib/logger";
import { getCrawl, saveCrawl } from "../../../src/lib/crawl-redis";
import {
getCrawl,
saveCrawl,
isCrawlKickoffFinished,
} from "../../../src/lib/crawl-redis";
import * as Sentry from "@sentry/node";
import { configDotenv } from "dotenv";
import { redisEvictConnection } from "../../../src/services/redis";
configDotenv();

async function isCrawlFinished(id: string) {
mogery marked this conversation as resolved.
Outdated Show resolved Hide resolved
await redisEvictConnection.expire(
"crawl:" + id + ":kickoff:finish",
24 * 60 * 60,
);
return (
(await redisEvictConnection.scard("crawl:" + id + ":jobs_done")) ===
(await redisEvictConnection.scard("crawl:" + id + ":jobs")) &&
(await isCrawlKickoffFinished(id))
);
}

export async function crawlCancelController(req: Request, res: Response) {
try {
const auth = await authenticateUser(req, res, RateLimiterMode.CrawlStatus);
Expand Down Expand Up @@ -53,6 +69,17 @@ export async function crawlCancelController(req: Request, res: Response) {
return res.status(403).json({ error: "Unauthorized" });
}

try {
const isCompleted = await isCrawlFinished(req.params.jobId);
if (isCompleted) {
return res.status(409).json({
error: "Cannot cancel job that has already completed",
});
}
} catch (error) {
logger.error("Error checking crawl completion status", error);
micahstairs marked this conversation as resolved.
Outdated Show resolved Hide resolved
}

try {
sc.cancelled = true;
await saveCrawl(req.params.jobId, sc);
Expand Down
30 changes: 29 additions & 1 deletion 30 apps/api/src/controllers/v1/crawl-cancel.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,11 +1,28 @@
import { Response } from "express";
import { logger } from "../../lib/logger";
import { getCrawl, saveCrawl } from "../../lib/crawl-redis";
import {
getCrawl,
saveCrawl,
isCrawlKickoffFinished,
} from "../../lib/crawl-redis";
import * as Sentry from "@sentry/node";
import { configDotenv } from "dotenv";
import { RequestWithAuth } from "./types";
import { redisEvictConnection } from "../../services/redis";
configDotenv();

async function isCrawlFinished(id: string) {
await redisEvictConnection.expire(
"crawl:" + id + ":kickoff:finish",
24 * 60 * 60,
);
return (
(await redisEvictConnection.scard("crawl:" + id + ":jobs_done")) ===
(await redisEvictConnection.scard("crawl:" + id + ":jobs")) &&
(await isCrawlKickoffFinished(id))
);
}

export async function crawlCancelController(
req: RequestWithAuth<{ jobId: string }>,
res: Response,
Expand All @@ -20,6 +37,17 @@ export async function crawlCancelController(
return res.status(403).json({ error: "Unauthorized" });
}

try {
const isCompleted = await isCrawlFinished(req.params.jobId);
if (isCompleted) {
return res.status(409).json({
error: "Cannot cancel job that has already completed",
});
}
} catch (error) {
logger.error("Error checking crawl completion status", error);
}

try {
sc.cancelled = true;
await saveCrawl(req.params.jobId, sc);
Expand Down
30 changes: 29 additions & 1 deletion 30 apps/api/src/controllers/v2/crawl-cancel.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,11 +1,28 @@
import { Response } from "express";
import { logger } from "../../lib/logger";
import { getCrawl, saveCrawl } from "../../lib/crawl-redis";
import {
getCrawl,
saveCrawl,
isCrawlKickoffFinished,
} from "../../lib/crawl-redis";
import * as Sentry from "@sentry/node";
import { configDotenv } from "dotenv";
import { RequestWithAuth } from "./types";
import { redisEvictConnection } from "../../services/redis";
configDotenv();

async function isCrawlFinished(id: string) {
mogery marked this conversation as resolved.
Outdated Show resolved Hide resolved
await redisEvictConnection.expire(
"crawl:" + id + ":kickoff:finish",
24 * 60 * 60,
);
return (
(await redisEvictConnection.scard("crawl:" + id + ":jobs_done")) ===
(await redisEvictConnection.scard("crawl:" + id + ":jobs")) &&
(await isCrawlKickoffFinished(id))
);
}

export async function crawlCancelController(
req: RequestWithAuth<{ jobId: string }>,
res: Response,
Expand All @@ -21,6 +38,17 @@ export async function crawlCancelController(
return res.status(403).json({ error: "Unauthorized" });
}

try {
const isCompleted = await isCrawlFinished(req.params.jobId);
if (isCompleted) {
return res.status(409).json({
error: "Cannot cancel job that has already completed",
});
}
} catch (error) {
logger.error("Error checking crawl completion status", error);
}

try {
sc.cancelled = true;
await saveCrawl(req.params.jobId, sc);
Expand Down
32 changes: 32 additions & 0 deletions 32 apps/api/v1-openapi.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -378,6 +378,22 @@
}
}
},
"409": {
"description": "Job already completed",
"content": {
"application/json": {
"schema": {
"type": "object",
"properties": {
"error": {
"type": "string",
"example": "Cannot cancel job that has already completed"
}
}
}
}
}
},
"500": {
"description": "Server error",
"content": {
Expand Down Expand Up @@ -607,6 +623,22 @@
}
}
},
"409": {
"description": "Job already completed",
"content": {
"application/json": {
"schema": {
"type": "object",
"properties": {
"error": {
"type": "string",
"example": "Cannot cancel job that has already completed"
}
}
}
}
}
},
"500": {
"description": "Server error",
"content": {
Expand Down
Loading
Morty Proxy This is a proxified and sanitized view of the page, visit original site.