Fix/stow response fix #111
Conversation
…rect-pixel-representation
| import { StreamInfo } from '../lib/instance/StreamInfo.mjs'; | ||
|
|
||
| describe('StreamInfo write and end', () => { | ||
| test('writes Buffer, array of Buffer and TypedArray via write(); returns backpressure; queues when busy', async () => { |
There was a problem hiding this comment.
This test only validates negative backpressure (returning true): there isn't a case to validate when write returns false
There was a problem hiding this comment.
I'm not quite sure how to validate positive backpressure without really hitting the stream hard, and it is hard to know how hard that has to be to work.
wayfarer3130
force-pushed
the
fix/stow-response-fix
branch
from
63c9fb1 to
29953cc
Compare
| return Promise.resolve(promise).catch(error => { | ||
| // Error should already be recorded internally by _writeToStream, but catch | ||
| // any unexpected rejections to prevent process termination | ||
| const streamKey = streamInfo?.streamKey || 'unknown'; |
There was a problem hiding this comment.
streamKey should be guaranteed to have a value due to the guard above on line 86, unless the streamInfo can be mutated to remove it
| }); | ||
| } catch (syncError) { | ||
| // Handle any synchronous errors (shouldn't happen since _writeToStream is async, but safety net) | ||
| const streamKey = streamInfo?.streamKey || 'unknown'; |
There was a problem hiding this comment.
same here: streamKey should be guaranteed to have a value
| }; | ||
|
|
||
| const onSettle = () => { | ||
| check(); |
There was a problem hiding this comment.
Could we simplify by moving the content of check into onSettle, or is there a reason they need to be separate functions? I see check returns a boolean while onSettle does not, but that does not look like it would change anything for the callers.
Fixes a number of crash issues caused by not handling promise rejections correclty and various other issues.