fix: WebTransport stream now extends abstract stream #2514
Conversation
The PR pulls all of the non-`@fails/webtransport` parts out of There's a lot of work that's been done to re-use existing libp2p code such as the abstract stream class which handles a lot more closing scenarios than the existing implementation so it would be good to get that in.
| if (!existsSync('./go-libp2p-webtransport-server/main')) { | ||
| await new Promise((resolve, reject) => { | ||
| exec('go build -o main main.go', | ||
| exec(`go build -o ${main} main.go`, |
There was a problem hiding this comment.
I don't think these build files are being cleaned by npm run clean, which they probably should be
There was a problem hiding this comment.
TBH I want to remove this transport server entirely and just rely on precomplied binaries in the interop tests because they exercise more functionality which gives us more certainty and we don't need to be recompiling this unchanging go script over and over again.
| if (result.done) { | ||
| init.log('remote closed write') | ||
| return | ||
| } | ||
|
|
||
| async closeWrite (options?: AbortOptions) { | ||
| if (!writerClosed) { | ||
| writerClosed = true | ||
| if (result.value != null) { | ||
| this.sourcePush(new Uint8ArrayList(result.value)) | ||
| } |
There was a problem hiding this comment.
Can value exist and the read be done? we should check for value first.
There was a problem hiding this comment.
Not exactly - if value exists and done is false, then value is the return value of the iterator, not a final value from the sequence.
See the iterator protocol docs: https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Iteration_protocols
There was a problem hiding this comment.
ah. then i've incorrectly implemented iterators previously, or consumed some incorrectly implemented ones, because i've seen a sequence value and a done=true... good to know that's not a correct implementation.
The PR pulls all of the non-
@fails/webtransportparts out of #2422There's a lot of work that's been done to re-use existing libp2p code such as the abstract stream class which handles a lot more closing scenarios than the existing implementation so it would be good to get that in.
Change checklist