Conversation
…gingServiceV2 exceeded 600000 milliseconds
product-auto-label
Bot
added
size: xs
| removeCircular?: boolean; | ||
| maxEntrySize?: number; // see: https://cloud.google.com/logging/quotas | ||
| jsonFieldsToTruncate?: string[]; | ||
| defaultWriteDeleteCallback?: ApiResponseCallback; |
There was a problem hiding this comment.
Is this global callback only for WriteDelete? I thought you want to have a global callback for all methods in Log class in manual layer.
There was a problem hiding this comment.
It is for write() and delete() APIs which are the only one's used to update logs. Just wanted to distinguish it to make sure we do not confuse developers. Perhaps naming WriteDelete is confusing? I can make it defaultWriteCallback to emphasize the "writing"
There was a problem hiding this comment.
a small correction:delete() does not update logs, it deletes all log entries for a provided log name as a result of the delete log operation while write() creates one or more log entries.
i am not sure about the "global" scope. the scope is bound by the Log instance, isn't it?
minherz
left a comment
minherz
left a comment
There was a problem hiding this comment.
From the issue (#1185) it is not clear what developers except to get. The complains sound like they do not want an application to crash in a case the retries fail to ingest the log.
In this respect, the fix does not resolve the problem if the callback is not provided.
| removeCircular?: boolean; | ||
| maxEntrySize?: number; // see: https://cloud.google.com/logging/quotas | ||
| jsonFieldsToTruncate?: string[]; | ||
| defaultWriteDeleteCallback?: ApiResponseCallback; |
There was a problem hiding this comment.
a small correction:delete() does not update logs, it deletes all log entries for a provided log name as a result of the delete log operation while write() creates one or more log entries.
i am not sure about the "global" scope. the scope is bound by the Log instance, isn't it?
Based on user's feedback in a bug, the suggestion was: "I assume a lot of users will want to set up some sort of global error catching for this particular error." |
product-auto-label
Bot
added
size: s
|
The quoted suggestion comes from one out of four people who describe scenarios that generally caused by failure in retrying mechanism due to transient connectivity error or because the ran their application in the not connected environment. One of the cases resulted in printing the error to STDOUT where it was catched by a logging agent and eventually reported to Cloud Logging backend. In my opinion the problem is when this error crashes the application. It is a good idea to provide a way to define the error callback. I suggest to add a default error handling if nothing is provided, so the application would not crash. |
I believe we cannot provide default callback since client may call |
No region tags are edited in this PR.This comment is generated by snippet-bot.
|
|
Warning: This pull request is touching the following templated files:
|
from 9.0.0 to 9.6.9 Error "GoogleError: Total timeout of API google.logging.v2.LoggingServiceV2 exceeded 60000 milliseconds before any response was received" has been fixed in version9.6.9 of @google-cloud/logging: googleapis/nodejs-logging#1185 googleapis/nodejs-logging#1225
4 participants
This is a proposed fix to accept a global callback for
Log.deleteandLog.writemethods throughLogOptionsFixes #<1185> 🦕