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

make sure get request_id before body chunk #5611

Merged
merged 1 commit into from
Oct 9, 2024
Merged

make sure get request_id before body chunk #5611

Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
37 changes: 20 additions & 17 deletions app/utils/stream.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,8 @@ export function fetch(url: string, options?: RequestInit): Promise<any> {
body = [],
} = options || {};
let unlisten: Function | undefined;
let request_id = 0;
let setRequestId: Function | undefined;
const requestIdPromise = new Promise((resolve) => (setRequestId = resolve));
Comment on lines +31 to +32
Copy link
Contributor

Choose a reason for hiding this comment

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

⚠️ Potential issue

Refactor to avoid using 'Function' type and assignment within expression

The use of Function as a type is discouraged because it lacks specificity and can lead to type safety issues. Additionally, assigning setRequestId within the expression can be confusing and may lead to unintended side effects.

Apply this diff to define the function type explicitly and refactor the assignment:

-let setRequestId: Function | undefined;
-const requestIdPromise = new Promise((resolve) => (setRequestId = resolve));
+let setRequestId!: (requestId: number) => void;
+const requestIdPromise = new Promise<number>((resolve) => {
+  setRequestId = resolve;
+});

This change:

  • Defines setRequestId with a specific function type (requestId: number) => void.
  • Moves the assignment of setRequestId outside of the expression for clarity.
  • Ensures better type safety and readability.
πŸ“ Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
let setRequestId: Function | undefined;
const requestIdPromise = new Promise((resolve) => (setRequestId = resolve));
let setRequestId!: (requestId: number) => void;
const requestIdPromise = new Promise<number>((resolve) => {
setRequestId = resolve;
});
🧰 Tools
πŸͺ› Biome

[error] 32-32: The assignment should not be in an expression.

The use of assignments in expressions is confusing.
Expressions are often considered as side-effect free.

(lint/suspicious/noAssignInExpressions)

[error] 31-31: Don't use 'Function' as a type.

Prefer explicitly define the function shape. This type accepts any function-like value, which can be a common source of bugs.

(lint/complexity/noBannedTypes)

const ts = new TransformStream();
const writer = ts.writable.getWriter();

Expand All @@ -47,20 +48,22 @@ export function fetch(url: string, options?: RequestInit): Promise<any> {
}
// @ts-ignore 2. listen response multi times, and write to Response.body
window.__TAURI__.event
.listen("stream-response", (e: ResponseEvent) => {
const { request_id: rid, chunk, status } = e?.payload || {};
if (request_id != rid) {
return;
}
if (chunk) {
writer.ready.then(() => {
writer.write(new Uint8Array(chunk));
});
} else if (status === 0) {
// end of body
close();
}
})
.listen("stream-response", (e: ResponseEvent) =>
requestIdPromise.then((request_id) => {
const { request_id: rid, chunk, status } = e?.payload || {};
if (request_id != rid) {
return;
}
if (chunk) {
writer.ready.then(() => {
writer.write(new Uint8Array(chunk));
});
} else if (status === 0) {
// end of body
close();
}
}),
)
.then((u: Function) => (unlisten = u));

const headers: Record<string, string> = {
Expand All @@ -83,8 +86,8 @@ export function fetch(url: string, options?: RequestInit): Promise<any> {
: [],
})
.then((res: StreamResponse) => {
request_id = res.request_id;
const { status, status_text: statusText, headers } = res;
const { request_id, status, status_text: statusText, headers } = res;
setRequestId?.(request_id);
const response = new Response(ts.readable, {
status,
statusText,
Expand Down
Loading