Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

On API error stream exits and doesn't recover #524

Closed
jmatsumura opened this issue Apr 9, 2021 · 2 comments
Closed

On API error stream exits and doesn't recover #524

jmatsumura opened this issue Apr 9, 2021 · 2 comments
Assignees
Labels
api: logging Issues related to the googleapis/nodejs-logging-bunyan API. lang: nodejs Issues specific to JavaScript or TypeScript. priority: p2 Moderately-important priority. Fix may not be included in next release. type: feature request ‘Nice-to-have’ improvement, new feature or different behavior or design.

Comments

@jmatsumura
Copy link

Environment details

  • OS: Docker container for launcher.gcr.io/google/nodejs (Ubuntu 16)
  • Node.js version: v14.16.0
  • npm version: 6.14.11
  • @google-cloud/logging-bunyan version: 3.0.2
  • bunyan version: 1.8.15

Steps to reproduce

  1. given the following snippet in a file called script.ts:
import { LoggingBunyan } from '@google-cloud/logging-bunyan';
import * as bunyan from 'bunyan';
import * as fs from 'fs';

function getLogger(className: string): bunyan {
  return bunyan.createLogger({
    name: className,
    streams: [
      new LoggingBunyan({
        projectId: 'PROJECT_ID',
        keyFilename: 'PATH_TO_CREDS_JSON',
      }).stream('trace'),
    ],
  });
}

const myLog = getLogger('james-local');

myLog.info({ 'test-1': true });
// myLog.info({ 'test-2': fs.readFileSync('temp_file.json', 'utf8') });
myLog.info({ 'test-3': true });
  1. replace PROJECT_ID and PATH_TO_CREDS_JSON with real values so we can talk to stackdriver locally
  2. run the code without an API error occurring and see no issues logging the myLog.info({ 'test-3': true }); e.g. ts-node script.ts
  3. generate a file slightly larger than the 256K limit to force an API error e.g. mkfile -n 260k temp_file.json
  4. uncomment the line that attempts to log the contents of this file myLog.info({ 'test-2': fs.readFileSync('temp_file.json', 'utf8') });
  5. run the code with an API error occurring and see issues logging, the process exits and never hits the myLog.info({ 'test-3': true }); and results in a stack trace like:
/${PATH_TO_PKG}/node_modules/@grpc/grpc-js/src/call.ts:81
  return Object.assign(new Error(message), status);
                       ^
Error: 3 INVALID_ARGUMENT: Log entry with size 260.3K exceeds maximum size of 256.0K
    at Object.callErrorFromStatus (/${PATH_TO_PKG}/node_modules/@grpc/grpc-js/src/call.ts:81:24)
    at Object.onReceiveStatus (/${PATH_TO_PKG}/node_modules/@grpc/grpc-js/src/client.ts:334:36)
    at Object.onReceiveStatus (/${PATH_TO_PKG}/node_modules/@grpc/grpc-js/src/client-interceptors.ts:426:34)
    at Object.onReceiveStatus (/${PATH_TO_PKG}/node_modules/@grpc/grpc-js/src/client-interceptors.ts:389:48)
    at /${PATH_TO_PKG}/node_modules/@grpc/grpc-js/src/call-stream.ts:276:24
    at processTicksAndRejections (internal/process/task_queues.js:75:11)

while there are ways to pre-empt this specific API error of exceeding the size limit, would like a way to make sure the stream can resume after this type of event occurs.

@product-auto-label product-auto-label bot added the api: logging Issues related to the googleapis/nodejs-logging-bunyan API. label Apr 9, 2021
@yoshi-automation yoshi-automation added triage me I really want to be triaged. 🚨 This issue needs some love. labels Apr 10, 2021
@keenan-devrel keenan-devrel added lang: nodejs Issues specific to JavaScript or TypeScript. type: feature request ‘Nice-to-have’ improvement, new feature or different behavior or design. priority: p2 Moderately-important priority. Fix may not be included in next release. labels Apr 20, 2021
@yoshi-automation yoshi-automation removed triage me I really want to be triaged. 🚨 This issue needs some love. labels Apr 21, 2021
@freelerobot freelerobot assigned simonz130 and unassigned freelerobot Jun 29, 2021
@simonz130 simonz130 assigned losalex and unassigned simonz130 Feb 14, 2022
@losalex
Copy link
Contributor

losalex commented Mar 2, 2022

@jmatsumura,
Sorry for long wait and thanks for filing this issue and for your patience! I must admit that details in description are very good!
Looking into issue, it has 2 "sub-problems" - truncating long entries and issue with handling errors.
As for truncation, we had a bug maxEntrySize does not truncate big json payloads correctly in nodejs-logging library which we solved in #1177. Thats said, we still did not integrated the fix with nodejs-logging-bunyan, so if you think we should expose this functionality in nodejs-logging-bunyan please let us know.
As for error handling, I believe this issue should be resolved by #601 - the solution is to provide a global callback which could be used to process responses and catch any errors. There is a sample here which could be used as reference on how such callback can be provided.
Please share your feedback and let us know if it helps or you have any other concerns.

@losalex
Copy link
Contributor

losalex commented Mar 15, 2022

@jmatsumura , closing this issue for now - please let me know if there is anything we can do and feel free to comment or reactivate an issue. Thanks!

# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
api: logging Issues related to the googleapis/nodejs-logging-bunyan API. lang: nodejs Issues specific to JavaScript or TypeScript. priority: p2 Moderately-important priority. Fix may not be included in next release. type: feature request ‘Nice-to-have’ improvement, new feature or different behavior or design.
Projects
None yet
Development

No branches or pull requests

6 participants