Skip to content
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

fix(js-sdk): Add CJS-compatible WebSocket loading #1021

Closed
wants to merge 5 commits into from
Closed

fix(js-sdk): Add CJS-compatible WebSocket loading #1021

Changes from 1 commit
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
cbf3730
fix(js-sdk): Add CJS-compatible WebSocket loading
RutamBhagat Dec 28, 2024
15398cc
fix(js-sdk): WebSocket initialization handles promises correctly
RutamBhagat Dec 30, 2024
4238270
fix(js-sdk): fix loadWebSocket race condition
RutamBhagat Dec 31, 2024
950ceb9
fix(js-sdk): ensure WebSocket closes after initialization completes
RutamBhagat Dec 31, 2024
67db6a1
fix(build): resolve CJS/ESM WebSocket import with build-time code inj…
RutamBhagat Jan 6, 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
17 changes: 15 additions & 2 deletions apps/js-sdk/firecrawl/src/index.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,9 +1,9 @@
import axios, { type AxiosResponse, type AxiosRequestHeaders, AxiosError } from "axios";
import type * as zt from "zod";
import { zodToJsonSchema } from "zod-to-json-schema";
import { WebSocket } from "isows";
import { TypedEventTarget } from "typescript-event-target";


/**
* Configuration interface for FirecrawlApp.
* @param apiKey - Optional API key for authentication.
Expand Down Expand Up @@ -947,10 +947,23 @@ export class CrawlWatcher extends TypedEventTarget<CrawlWatcherEvents> {
public status: CrawlStatusResponse["status"];
public id: string;

private static loadWebSocket() {
try {
return require('isows').WebSocket;
} catch {
try {
return import('isows').then(m => m.WebSocket);
Copy link
Member

Choose a reason for hiding this comment

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

Missing an await here, right? Either here or on line 964.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks for catching that! I've updated the code to properly handle the WebSocket loading. Could you please review the changes again?

Copy link
Member

Choose a reason for hiding this comment

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

This can cause a race condition. I'd prefer importing it lazily in an already-asynchronous function, like the watch crawl endpoint's function.

} catch {
throw new FirecrawlError("WebSocket module failed to load", 500);
}
}
}

constructor(id: string, app: FirecrawlApp) {
super();
const WS = CrawlWatcher.loadWebSocket();
this.id = id;
this.ws = new WebSocket(`${app.apiUrl}/v1/crawl/${id}`, app.apiKey);
this.ws = new WS(`${app.apiUrl}/v1/crawl/${id}`, app.apiKey);
this.status = "scraping";
this.data = [];

Expand Down
Loading