googleapis / googleapis/google-cloud-node
logging-winston: failed write crashes the process under default wiring; auth-time failure never invokes the completion callback (logger.end() hangs)
- Dominant language
- TypeScript
- Stars
- 3.2k
- Forks
- 712
- Avg merge
- 2d 3h
- Merged PRs (30d)
- 99
Description
## Environment
- `@google-cloud/logging-winston` 6.0.2 (latest at time of writing)
- `@google-cloud/logging` 11.3.0
- `winston` 3.x, Node.js v22.x
## Summary
Two failure-path defects that together make the transport unsafe for production services. Both were reproduced with real `winston.createLogger` + real `LoggingWinston` wiring (nothing manually `.emit()`'d); the write RPC was stubbed where noted so the failure mode is deterministic.
### 1. A single failed write crashes the host process with default wiring
With an API-level write rejection (e.g. `PERMISSION_DENIED` from `writeLogEntries`) and the transport attached the way the README shows (no logger-level `'error'` listener):
```
[transport "error" event fired] simulated PERMISSION_DENIED from writeLogEntries
Error: simulated PERMISSION_DENIED from writeLogEntries
at LoggingWinston._write (winston-transport/modern.js:82)
at doWrite (readable-stream/lib/_stream_writable.js:390)
Emitted 'error' event on DerivedLogger instance at:
at DerivedLogger.transportEvent (winston/lib/winston/logger.js:663:12)
Node.js v22.22.3
EXIT_CODE=1
```
Attaching `transport.on('error')` is **not** sufficient — winston's `Logger` re-emits the transport error on itself, and with no logger-level handler Node's default unhandled-`'error'` behavior kills the process. A logging transport whose ordinary failure mode (transient quota/permission/API errors) crashes the service it observes is a serious footgun; at minimum the README should require a `logger.on('error')` handler, but better would be for the transport to never invoke the winston callback with an error for fire-and-forget writes (report through its own `'error'` event only).
Reproduced matrix: with BOTH `transport.on('error')` and `logger.on('error')` attached the same rejection is handled cleanly (exit 0) — confirming the crash is exactly the missing-logger-listener path.
### 2. Auth-time failures never invoke the completion callback — `logger.end()` hangs forever
With `GOOGLE_APPLICATION_CREDENTIALS` pointing at a nonexistent file (a misconfiguration you'd want to fail loudly but catchably):
```
[harness] logger.end() callback did NOT fire within 5000ms
RESULT_JSON={"transportErrorFired":false,"loggerErrorFired":false,"endCallbackFired":false,
"unhandledRejectionSeen":null,"uncaughtExceptionSeen":"...ENOENT..."}
```
The process gets an `uncaughtException` from lazy stub creation (that part is an upstream gax defect — filed separately as https://github.com/googleapis/google-cloud-node/issues/9085), but the transport-level consequence stands on its own: the winston completion callback for the in-flight write never fires, so `logger.end()` (and any graceful-shutdown flush built on it) hangs indefinitely. The success path is fine (callback fires via the promisify chain) — the gap is specifically the failure path.
## Expected
- A rejected write must never crash the host process under the documented default wiring.
- The winston completion callback must fire exactly once for every write, success **or** failure, so `logger.end()` semantics hold.
## Workaround
We replaced the transport with a ~180-line custom `winston-transport` that calls `Log.write(entry).catch(note).then(() => callback())` — the callback always fires and never carries an error, so the crash-prone `'error'`-forwarding path is unreachable by construction. Happy to share details.
Contributor guide
Research direction
Start at LoggingWinston._write and trace its winston-transport callback and error handling, then compare the transport behavior with winston's Logger.transportEvent and logger.end() flow. Reproduce both cases using real winston.createLogger and LoggingWinston wiring, including the stubbed write rejection and missing credentials. Done means rejected writes do not crash the default setup and every write invokes the completion callback exactly once.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- nodejs, typescript
- Domain
- backend, observability-sre
- Issue type
- Bug
- Difficulty
- 4/5
- Estimated time
- 3-5 days
- Activity status
- Quiet
- Clarity
- Mostly clear
- Newbie friendliness
- 52/100