Skip to content

exposing file writable stream errors (from #316)#520

Closed
tunnckoCore wants to merge 2 commits into
masterfrom
writable-stream-errors
Closed

exposing file writable stream errors (from #316)#520
tunnckoCore wants to merge 2 commits into
masterfrom
writable-stream-errors

Conversation

@tunnckoCore

@tunnckoCore tunnckoCore commented Nov 28, 2019

Copy link
Copy Markdown
Member

thanks to #316 by @gabipetrovay, closes #469, possible fix #470 too

tip on merge: add Co-authored-by: Gabriel Petrovay <gabipetrovay@gmail.com>

@tunnckoCore tunnckoCore marked this pull request as ready for review November 28, 2019 04:06
@tunnckoCore

Copy link
Copy Markdown
Member Author

Yea... of course, it breaks the "legacy" tests :D

@tunnckoCore

tunnckoCore commented Nov 28, 2019

Copy link
Copy Markdown
Member Author

Okay, i don't get it. Even with fs.createWriteStream the this._writeStream is undefined 😱 WAT?!

But in anyway... everything boils down back to that we should rewrite tests without stubbing and gently. I'm presonally not fan of stubs.

Comment thread lib/file.js
util.inherits(File, EventEmitter);

File.prototype.open = function() {
var self = this;

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe we can use an array function here to omit using a self variable?

@tunnckoCore

Copy link
Copy Markdown
Member Author

linking for reminder to come back here: #545 & #415

@tunnckoCore

tunnckoCore commented Jan 29, 2020

Copy link
Copy Markdown
Member Author

closing in b70bb80

@tunnckoCore tunnckoCore deleted the writable-stream-errors branch January 29, 2020 18:20
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Return the transmission when the large file is uploaded Crash when aborting a file too soon

3 participants