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.

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

chore(log): rework std/log to work without deprecated Deno.Writer #4021

Merged
merged 4 commits into from
Dec 28, 2023

Conversation

syhol
Copy link
Contributor

@syhol syhol commented Dec 24, 2023

Attempting to solve #3907

std/log is the last of the sub-modules that uses the deprecated Writer (i.e. Deno.Writer) interface. It should use WritableStream instead.

Previous attempt of #3969 was trying to use std/streams/buffer.ts and the streams API, but moving to async flushes resulted in potentially lost logs.

Now just solving with a Uint8Array and a ArrayBuffer and a number pointer.

@syhol syhol requested a review from kt3k as a code owner December 24, 2023 01:27
@github-actions github-actions bot added the log label Dec 24, 2023
log/handlers.ts Outdated Show resolved Hide resolved
log/handlers.ts Outdated Show resolved Hide resolved
@syhol
Copy link
Contributor Author

syhol commented Dec 28, 2023 via email

@iuioiua
Copy link
Contributor

iuioiua commented Dec 28, 2023

Hash prefix please 🙂

syhol and others added 2 commits December 28, 2023 01:21
Co-authored-by: Asher Gomez <ashersaupingomez@gmail.com>
@syhol
Copy link
Contributor Author

syhol commented Dec 28, 2023

All suggestions applied 🙏 ready for another review 🎉

@iuioiua iuioiua changed the title chore(log): rework std/log to no longer depend on the deprecated Writer chore(log): rework std/log to work without deprecated Deno.Writer Dec 28, 2023
Copy link
Contributor

@iuioiua iuioiua left a comment

Choose a reason for hiding this comment

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

Great work! Thank you for this.

@iuioiua iuioiua merged commit 835480d into denoland:main Dec 28, 2023
12 checks passed
@dvv
Copy link

dvv commented Feb 28, 2024

Hi!

With this attempt to log circa 4096 octets message just dumps the process.

import { RotatingFileHandler } from "https://deno.land/std@0.217.0/log/mod.ts"

const logger = new RotatingFileHandler(
  "DEBUG",
  {
    filename: "/tmp/123",
    maxBytes: 4000000,
    maxBackupCount: 10,
  },
)
logger.log("x".repeat(4096))
error: Uncaught (in promise) RangeError: offset is out of bounds
    this._buf.set(bytes, this._pointer);
              ^
    at Uint8Array.set (<anonymous>)
    at RotatingFileHandler.log (https://deno.land/std@0.217.0/log/file_handler.ts:79:15)
    at RotatingFileHandler.log (https://deno.land/std@0.217.0/log/rotating_file_handler.ts:106:11)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants