Skip to content

chore: remove console logs #262

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 1 commit into
base: main
Choose a base branch
from
Open

chore: remove console logs #262

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
58 changes: 29 additions & 29 deletions packages/partyserver/src/index.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -561,9 +561,9 @@ Did you try connecting directly to this Durable Object? Try using getServerByNam
connection: Connection,
ctx: ConnectionContext
): void | Promise<void> {
console.log(
`Connection ${connection.id} connected to ${this.#ParentClass.name}:${this.name}`
);
// console.log(
Copy link
Collaborator

@threepointone threepointone Aug 18, 2025

Choose a reason for hiding this comment

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

this one I could be convinced, but it is causing a problem?

// `Connection ${connection.id} connected to ${this.#ParentClass.name}:${this.name}`
// );
// console.log(
// `Implement onConnect on ${this.#ParentClass.name} to handle websocket connections.`
// );
Expand All @@ -573,12 +573,12 @@ Did you try connecting directly to this Durable Object? Try using getServerByNam
* Called when a message is received from a connection.
*/
onMessage(connection: Connection, message: WSMessage): void | Promise<void> {
console.log(
`Received message on connection ${this.#ParentClass.name}:${connection.id}`
);
console.info(
`Implement onMessage on ${this.#ParentClass.name} to handle this message.`
);
// console.log(
Copy link
Collaborator

@threepointone threepointone Aug 18, 2025

Choose a reason for hiding this comment

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

receiving messages without handling them?

// `Received message on connection ${this.#ParentClass.name}:${connection.id}`
// );
// console.info(
// `Implement onMessage on ${this.#ParentClass.name} to handle this message.`
// );
}

/**
Expand All @@ -595,13 +595,13 @@ Did you try connecting directly to this Durable Object? Try using getServerByNam
* Called when an error occurs on a connection.
*/
onError(connection: Connection, error: unknown): void | Promise<void> {
console.error(
`Error on connection ${connection.id} in ${this.#ParentClass.name}:${this.name}:`,
error
);
console.info(
`Implement onError on ${this.#ParentClass.name} to handle this error.`
);
// console.error(
Copy link
Collaborator

@threepointone threepointone Aug 18, 2025

Choose a reason for hiding this comment

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

same as onException, you probably don't want a websocket error that isn't handled

// `Error on connection ${connection.id} in ${this.#ParentClass.name}:${this.name}:`,
// error
// );
// console.info(
// `Implement onError on ${this.#ParentClass.name} to handle this error.`
// );
}

/**
Expand All @@ -610,9 +610,9 @@ Did you try connecting directly to this Durable Object? Try using getServerByNam
onRequest(request: Request): Response | Promise<Response> {
// default to 404

console.warn(
`onRequest hasn't been implemented on ${this.#ParentClass.name}:${this.name} responding to ${request.url}`
);
// console.warn(
Copy link
Collaborator

@threepointone threepointone Aug 18, 2025

Choose a reason for hiding this comment

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

we want to keep this one, or you'll get a request without a log, and won't know what to do here?

// `onRequest hasn't been implemented on ${this.#ParentClass.name}:${this.name} responding to ${request.url}`
// );

return new Response("Not implemented", { status: 404 });
}
Expand All @@ -622,19 +622,19 @@ Did you try connecting directly to this Durable Object? Try using getServerByNam
* @param error - The error that occurred.
*/
onException(error: unknown): void | Promise<void> {
console.error(
`Exception in ${this.#ParentClass.name}:${this.name}:`,
error
);
console.info(
`Implement onException on ${this.#ParentClass.name} to handle this error.`
);
// console.error(
Copy link
Collaborator

@threepointone threepointone Aug 18, 2025

Choose a reason for hiding this comment

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

we want to keep this one, no? odd to have an exception that you haven't caught or handled here

// `Exception in ${this.#ParentClass.name}:${this.name}:`,
// error
// );
// console.info(
// `Implement onException on ${this.#ParentClass.name} to handle this error.`
// );
}

onAlarm(): void | Promise<void> {
console.log(
`Implement onAlarm on ${this.#ParentClass.name} to handle alarms.`
);
// console.log(
Copy link
Collaborator

@threepointone threepointone Aug 18, 2025

Choose a reason for hiding this comment

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

we want to keep this one, no? would be weird to set an alarm and not have overridden this

// `Implement onAlarm on ${this.#ParentClass.name} to handle alarms.`
// );
}

async alarm(): Promise<void> {
Expand Down