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(log): don't discard bytes >4096 in FileHandler #4415

Merged
merged 8 commits into from
Mar 1, 2024
Merged

fix(log): don't discard bytes >4096 in FileHandler #4415

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
8 commits
Select commit Hold shift + click to select a range
bc813ae
fix(log): fix error when logging >4096 bytes when using FileHandler o…
syhol Feb 29, 2024
e6ea1bb
trigger ci
syhol Feb 29, 2024
62dedeb
Update log/rotating_file_handler_test.ts
syhol Feb 29, 2024
2f8c585
Update log/rotating_file_handler_test.ts
syhol Feb 29, 2024
114b5c8
Update log/file_handler_test.ts
syhol Feb 29, 2024
b910cdc
Update log/file_handler_test.ts
syhol Feb 29, 2024
d109446
Merge branch 'main' into fix-error-on-large-logs
syhol Feb 29, 2024
b57f871
Merge branch 'main' into fix-error-on-large-logs
kt3k Feb 29, 2024
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
9 changes: 7 additions & 2 deletions log/file_handler.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@
import { type LevelName, LogLevels } from "./levels.ts";
import type { LogRecord } from "./logger.ts";
import { BaseHandler, type BaseHandlerOptions } from "./base_handler.ts";
import { writeAllSync } from "../io/write_all.ts";

const PAGE_SIZE = 4096;
export type LogMode = "a" | "w" | "x";
Expand Down Expand Up @@ -76,8 +77,12 @@ export class FileHandler extends BaseHandler {
if (bytes.byteLength > this._buf.byteLength - this._pointer) {
this.flush();
}
this._buf.set(bytes, this._pointer);
this._pointer += bytes.byteLength;
if (bytes.byteLength > this._buf.byteLength) {
writeAllSync(this._file!, bytes);
} else {
this._buf.set(bytes, this._pointer);
this._pointer += bytes.byteLength;
}
}

flush() {
Expand Down
49 changes: 49 additions & 0 deletions log/file_handler_test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -162,3 +162,52 @@ Deno.test({
Deno.removeSync(LOG_FILE);
},
});

Deno.test({
name: "FileHandler handles strings larger than the buffer",
fn() {
const fileHandler = new FileHandler("WARN", {
filename: LOG_FILE,
mode: "w",
});
const logOverBufferLimit = "A".repeat(4096);
fileHandler.setup();

fileHandler.log(logOverBufferLimit);
fileHandler.destroy();

const decoder = new TextDecoder();
assertEquals(
decoder.decode(Deno.readFileSync(LOG_FILE)),
syhol marked this conversation as resolved.
Show resolved Hide resolved
`${logOverBufferLimit}\n`,
);

Deno.removeSync(LOG_FILE);
},
});

Deno.test({
name: "FileHandler handles a mixture of string sizes",
fn() {
const fileHandler = new FileHandler("WARN", {
filename: LOG_FILE,
mode: "w",
});
const veryLargeLog = "A".repeat(10000);
const regularLog = "B".repeat(100);
fileHandler.setup();

fileHandler.log(regularLog);
fileHandler.log(veryLargeLog);
fileHandler.log(regularLog);
fileHandler.destroy();

const decoder = new TextDecoder();
assertEquals(
decoder.decode(Deno.readFileSync(LOG_FILE)),
syhol marked this conversation as resolved.
Show resolved Hide resolved
`${regularLog}\n${veryLargeLog}\n${regularLog}\n`,
);

Deno.removeSync(LOG_FILE);
},
});
53 changes: 53 additions & 0 deletions log/rotating_file_handler_test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -298,3 +298,56 @@ Deno.test({
Deno.removeSync(LOG_FILE + ".1");
},
});

Deno.test({
name: "RotatingFileHandler handles strings larger than the buffer",
fn() {
const fileHandler = new RotatingFileHandler("WARN", {
filename: LOG_FILE,
mode: "w",
maxBytes: 4000000,
maxBackupCount: 10,
});
const logOverBufferLimit = "A".repeat(4096);
fileHandler.setup();

fileHandler.log(logOverBufferLimit);
fileHandler.destroy();

const decoder = new TextDecoder();
assertEquals(
decoder.decode(Deno.readFileSync(LOG_FILE)),
syhol marked this conversation as resolved.
Show resolved Hide resolved
`${logOverBufferLimit}\n`,
);

Deno.removeSync(LOG_FILE);
},
});

Deno.test({
name: "RotatingFileHandler handles a mixture of string sizes",
fn() {
const fileHandler = new RotatingFileHandler("WARN", {
filename: LOG_FILE,
mode: "w",
maxBytes: 4000000,
maxBackupCount: 10,
});
const veryLargeLog = "A".repeat(10000);
const regularLog = "B".repeat(100);
fileHandler.setup();

fileHandler.log(regularLog);
fileHandler.log(veryLargeLog);
fileHandler.log(regularLog);
fileHandler.destroy();

const decoder = new TextDecoder();
assertEquals(
decoder.decode(Deno.readFileSync(LOG_FILE)),
syhol marked this conversation as resolved.
Show resolved Hide resolved
`${regularLog}\n${veryLargeLog}\n${regularLog}\n`,
);

Deno.removeSync(LOG_FILE);
},
});
Loading