Update logging client options documentation on ably.com #1766
Assignees
Labels
documentation
Improvements or additions to public interface documentation (API reference or readme).
Comments
lawrence-forooghian
added
the
documentation
lawrence-forooghian
added a commit
that referenced
this issue
May 17, 2024
Currently, if you create a client with logHandler and/or logLevel
options, these options will affect all existing client objects. This
means that code like the following doesn’t do what you’d expect — the
log messages emitted by firstClient will say "secondClient":
```
const firstClient = new Realtime({
logHandler: (msg) => console.log("firstClient: ", msg),
})
const secondClient = new Realtime({
logHandler: (msg) => console.log("secondClient: ", msg),
})
```
Here we fix this as much as we can within the limitations of the
existing public API (see usage of defaultLogger). I _think_ that using
the default logging config is the right thing to do in the situations
where we have a standalone function or static method that doesn’t accept
logging configuration, but you could also argue that we should instead
maintain global effects for these usages and use the last-set logging
configuration. I don’t think it hugely matters because we actually only
end up falling back to defaultLogger in:
- the tests (that’s fine, we’ll see the console output)
- reporting failure of functions that are directly invoked by the user
and which throw an error upon failure anyway (i.e. even if the log
message doesn’t go where the user expected, they’ll still be informed of
the event the message described)
I’ve kept the Logger.logAction* static methods instead of changing them
to instance methods, because that way they remain easy for our
strip-logs plugin to target.
Created #1766 for updating ably.com documentation.
Resolves #156.
lawrence-forooghian
added a commit
that referenced
this issue
May 17, 2024
Currently, if you create a client with logHandler and/or logLevel
options, these options will affect all existing client objects. This
means that code like the following doesn’t do what you’d expect — the
log messages emitted by firstClient will say "secondClient":
```
const firstClient = new Realtime({
logHandler: (msg) => console.log("firstClient: ", msg),
})
const secondClient = new Realtime({
logHandler: (msg) => console.log("secondClient: ", msg),
})
```
Here we fix this as much as we can within the limitations of the
existing public API (see usage of defaultLogger). I _think_ that using
the default logging config is the right thing to do in the situations
where we have a standalone function or static method that doesn’t accept
logging configuration, but you could also argue that we should instead
maintain global effects for these usages and use the last-set logging
configuration. I don’t think it hugely matters because we actually only
end up falling back to defaultLogger in:
- the tests (that’s fine, we’ll see the console output)
- reporting failure of functions that are directly invoked by the user
and which throw an error upon failure anyway (i.e. even if the log
message doesn’t go where the user expected, they’ll still be informed of
the event the message described)
I’ve kept the Logger.logAction* static methods instead of changing them
to instance methods, because that way they remain easy for our
strip-logs plugin to target.
Created #1766 for updating ably.com documentation.
Resolves #156.
lawrence-forooghian
added a commit
that referenced
this issue
May 23, 2024
Currently, if you create a client with logHandler and/or logLevel
options, these options will affect all existing client objects. This
means that code like the following doesn’t do what you’d expect — the
log messages emitted by firstClient will say "secondClient":
```
const firstClient = new Realtime({
logHandler: (msg) => console.log("firstClient: ", msg),
})
const secondClient = new Realtime({
logHandler: (msg) => console.log("secondClient: ", msg),
})
```
Here we fix this as much as we can within the limitations of the
existing public API (see usage of defaultLogger). I _think_ that using
the default logging config is the right thing to do in the situations
where we have a standalone function or static method that doesn’t accept
logging configuration, but you could also argue that we should instead
maintain global effects for these usages and use the last-set logging
configuration. I don’t think it hugely matters because we actually only
end up falling back to defaultLogger in:
- the tests (that’s fine, we’ll see the console output)
- reporting failure of functions that are directly invoked by the user
and which throw an error upon failure anyway (i.e. even if the log
message doesn’t go where the user expected, they’ll still be informed of
the event the message described)
I’ve kept the Logger.logAction* static methods instead of changing them
to instance methods, because that way they remain easy for our
strip-logs plugin to target.
Created #1766 for updating ably.com documentation.
Resolves #156.
lawrence-forooghian
added a commit
to ably/docs
that referenced
this issue
May 29, 2024
- `log` was split into `logLevel` and `logHandler` (to conform with the spec) in ably-js v2. - The global effect of setting logging configuration was removed in ably-js commit 5502a43 and will be released in 2.1.0. Resolves ably/ably-js#1766.
|
Opened ably/docs#2199 but need to figure out how to get it to deploy again, have asked internally. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
https://ably.com/docs/api/realtime-sdk?lang=javascript
loginstead oflogHandlerandlogLevel)logHandlerandlogLevel#1764┆Issue is synchronized with this Jira Task by Unito
The text was updated successfully, but these errors were encountered: