Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: bindings now error when closed during empty writes #1872

Merged
merged 1 commit into from
May 18, 2019
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions packages/binding-abstract/binding-abstract.js
Original file line number Diff line number Diff line change
Expand Up @@ -119,6 +119,8 @@ The in progress writes must error when the port is closed with an error object t

debug('write', buffer.length, 'bytes')
if (!this.isOpen) {
debug('write', 'error port is not open')

throw new Error('Port is not open')
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,17 @@ function disconnect(err) {
throw err || new Error('Unknown disconnection')
}

function shouldReject(promise, errType = Error, message = 'Should have rejected') {
return promise.then(
() => {
throw new Error(message)
},
err => {
assert.instanceOf(err, errType)
}
)
}

// All bindings are required to work with an "echo" firmware
// The echo firmware should respond with this data when it's
// ready to echo. This allows for remote device bootup.
Expand Down Expand Up @@ -156,22 +167,12 @@ function testBinding(bindingName, Binding, testPort) {
})
})

it('throws when not given a path', done => {
try {
binding.open('')
} catch (e) {
assert.instanceOf(e, TypeError)
done()
}
it('throws when not given a path', async () => {
await shouldReject(binding.open(''), TypeError)
})

it('throws when not given options', done => {
try {
binding.open('COMBAD')
} catch (e) {
assert.instanceOf(e, TypeError)
done()
}
it('throws when not given options', async () => {
await shouldReject(binding.open('COMBAD'), TypeError)
})

if (!testPort) {
Expand Down Expand Up @@ -278,15 +279,9 @@ function testBinding(bindingName, Binding, testPort) {
})

describe('#update', () => {
it('throws when not given an object', done => {
it('throws when not given an object', async () => {
const binding = new Binding({ disconnect })

try {
binding.update()
} catch (e) {
assert.instanceOf(e, TypeError)
done()
}
await shouldReject(binding.update(), TypeError)
})

it('errors asynchronously when not open', done => {
Expand Down Expand Up @@ -315,53 +310,47 @@ function testBinding(bindingName, Binding, testPort) {

afterEach(() => binding.close())

it('throws errors when updating nothing', done => {
try {
binding.update({})
} catch (err) {
assert.instanceOf(err, Error)
done()
}
it('throws errors when updating nothing', async () => {
await shouldReject(binding.update({}), Error)
})

it('errors when not called with options', done => {
try {
binding.set(() => {})
} catch (e) {
assert.instanceOf(e, Error)
done()
}
it('errors when not called with options', async () => {
await shouldReject(binding.set(() => {}), Error)
})

it('updates baudRate', () => {
return binding.update({ baudRate: 57600 })
})
})

describe('#write', () => {
describe.only('#write', () => {
it('errors asynchronously when not open', done => {
const binding = new Binding({
disconnect,
})
let noZalgo = false
binding.write(Buffer.from([])).catch(err => {
assert.instanceOf(err, Error)
assert(noZalgo)
done()
})
binding
.write(Buffer.from([]))
.then(
data => {
console.log({ data })
throw new Error('Should have errored')
},
err => {
assert.instanceOf(err, Error)
assert(noZalgo)
done()
}
)
.catch(done)
noZalgo = true
})

it('throws when not given a buffer', done => {
it('throws when not given a buffer', async () => {
const binding = new Binding({
disconnect,
})
try {
binding.write(null)
} catch (e) {
assert.instanceOf(e, TypeError)
done()
}
await shouldReject(binding.write(null), TypeError)
})

if (!testPort) {
Expand Down Expand Up @@ -494,16 +483,11 @@ function testBinding(bindingName, Binding, testPort) {
noZalgo = true
})

it('throws when not called with options', done => {
it('throws when not called with options', async () => {
const binding = new Binding({
disconnect,
})
try {
binding.set(() => {})
} catch (e) {
assert.instanceOf(e, TypeError)
done()
}
await shouldReject(binding.set(() => {}), TypeError)
})

if (!testPort) {
Expand Down
10 changes: 6 additions & 4 deletions packages/bindings/lib/darwin.js
Original file line number Diff line number Diff line change
Expand Up @@ -65,12 +65,14 @@ class DarwinBinding extends AbstractBinding {
}

async write(buffer) {
if (buffer.length === 0) {
return
}
this.writeOperation = super
.write(buffer)
.then(() => unixWrite.call(this, buffer))
.then(() => {
if (buffer.length === 0) {
return
}
return unixWrite.call(this, buffer)
})
.then(() => {
this.writeOperation = null
})
Expand Down
10 changes: 6 additions & 4 deletions packages/bindings/lib/linux.js
Original file line number Diff line number Diff line change
Expand Up @@ -65,12 +65,14 @@ class LinuxBinding extends AbstractBinding {
}

async write(buffer) {
if (buffer.length === 0) {
return
}
this.writeOperation = super
.write(buffer)
.then(() => unixWrite.call(this, buffer))
.then(() => {
if (buffer.length === 0) {
return
}
return unixWrite.call(this, buffer)
})
.then(() => {
this.writeOperation = null
})
Expand Down
10 changes: 6 additions & 4 deletions packages/bindings/lib/win32.js
Original file line number Diff line number Diff line change
Expand Up @@ -74,12 +74,14 @@ class WindowsBinding extends AbstractBinding {
}

async write(buffer) {
if (buffer.length === 0) {
return
}
this.writeOperation = super
.write(buffer)
.then(() => asyncWrite(this.fd, buffer))
.then(() => {
if (buffer.length === 0) {
return
}
return asyncWrite(this.fd, buffer)
})
.then(() => {
this.writeOperation = null
})
Expand Down