Jens-G commented on PR #3919: URL: https://github.com/apache/thrift/pull/3919#issuecomment-5761545788
### Code review No blocking issues found. Checked for bugs and CLAUDE.md compliance. Two suggestions, below the bar for an issue but verified: - A small formality for when you next push: CONTRIBUTING.md asks for a `Client: nodejs` line below the subject of the commit message. At the moment it is only in the PR description. https://github.com/apache/thrift/blob/e8b37a096737f0042c4aab64aa959c256c3802e8/CONTRIBUTING.md#L22-L27 - The generated code now calls `reset()` on the transport if it has one. `TBufferedTransport` does, but `TFramedTransport` does not, so framed clients still send the half-written message in front of the next request: after `testString(42)` throws, the next `testVoid()` goes out as one 50-byte frame that starts with the 25 leftover bytes. This was the same before this PR. Adding a `reset()` to `TFramedTransport` that clears `outBuffers` and `outCount` fixes it; with that, the next frame contains only `testVoid`. That could go into this PR or into a follow-up. https://github.com/apache/thrift/blob/e8b37a096737f0042c4aab64aa959c256c3802e8/compiler/cpp/src/thrift/generate/t_js_generator.cc#L2277-L2281 https://github.com/apache/thrift/blob/e8b37a096737f0042c4aab64aa959c256c3802e8/lib/nodejs/lib/thrift/buffered_transport.js#L39-L47 🤖 Generated with [Claude Code](https://claude.ai/code) -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
