Skip to content
This repository has been archived by the owner on Feb 13, 2024. It is now read-only.

#287 Dhaden/flush return rejectable promise #288

Closed
wants to merge 2 commits into from
Closed

#287 Dhaden/flush return rejectable promise #288

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
fca7772
Reject flush promise inside catch
Aug 1, 2021
f4d0941
Bumped ava and attempted to fix tests
Aug 2, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
4 changes: 3 additions & 1 deletion index.js
View file
Open in desktop
Expand Up @@ -289,10 +289,12 @@ class Analytics {
.catch(err => {
if (err.response) {
const error = new Error(err.response.statusText)
return done(error)
done(error)
throw error
}

done(err)
throw err
})
}

Expand Down
4 changes: 2 additions & 2 deletions package.json
View file
Open in desktop
Expand Up @@ -15,7 +15,7 @@
"scripts": {
"circle-lint": ".buildscript/circle.sh",
"dependencies": "yarn",
"test": "standard && nyc ava --timeout=20s&& .buildscript/e2e.sh",
"test": "standard && nyc ava --timeout=20s && .buildscript/e2e.sh",
"coverage": "nyc npm run test",
"report-coverage": "nyc report --reporter=lcov > coverage.lcov && codecov",
"np": "np --no-publish",
Expand Down Expand Up @@ -44,7 +44,7 @@
"uuid": "^8.3.2"
},
"devDependencies": {
"ava": "^0.25.0",
"ava": "^3.15.0",
"basic-auth": "^2.0.1",
"body-parser": "^1.17.1",
"codecov": "^3.8.1",
Expand Down
79 changes: 46 additions & 33 deletions test.js
View file
Open in desktop
@@ -1,12 +1,12 @@
import { spy, stub } from 'sinon'
import bodyParser from 'body-parser'
import express from 'express'
import delay from 'delay'
import auth from 'basic-auth'
import pify from 'pify'
import test from 'ava'
import Analytics from '.'
import { version } from './package'
const { spy, stub } = require('sinon')
Copy link
Contributor Author

@Dahaden Dahaden Aug 2, 2021 •
edited

Choose a reason for hiding this comment

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

Annoyingly babel was removed from ava and imports arent native until node 13 so have to revert to requires here

const bodyParser = require('body-parser')
const express = require('express')
const delay = require('delay')
const auth = require('basic-auth')
const pify = require('pify')
const test = require('ava')
const Analytics = require('./index.js')
const { version } = require('./package')

const noop = () => {}

Expand Down Expand Up @@ -88,7 +88,8 @@ test('expose a constructor', t => {
})

test('require a write key', t => {
t.throws(() => new Analytics(), 'You must pass your Segment project\'s write key.')
const error = t.throws(() => new Analytics(), { code: 'ERR_ASSERTION' })
t.is(error.message, 'You must pass your Segment project\'s write key.')
Copy link
Contributor Author

Choose a reason for hiding this comment

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

With latest versions of ava, you can no longer provide just a string.

I can make this consistent with the other tests, or use this method for testing exceptions. Either or.

})

test('create a queue', t => {
Expand Down Expand Up @@ -296,7 +297,7 @@ test('enqueue - skip when client is disabled', async t => {
test('flush - don\'t fail when queue is empty', async t => {
const client = createClient()

await t.notThrows(client.flush())
await t.notThrowsAsync(() => client.flush())
Copy link
Contributor Author

Choose a reason for hiding this comment

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

For anything that returns a promise, we have to use t.throwsAsync or t.notThrowsAsync now

})

test('flush - send messages', async t => {
Expand Down Expand Up @@ -342,7 +343,11 @@ test('flush - respond with an error', async t => {
}
]

await t.throws(client.flush(), 'Bad Request')
try {
await t.throwsAsync(() => client.flush())
} catch (error) {
console.log('Dafuq:', error)
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Part of debugging in an attempt to find where the unhandled promise rejection was coming from.
This console log is not called at all. Very confused

}
})

test('flush - time out if configured', async t => {
Expand All @@ -356,7 +361,12 @@ test('flush - time out if configured', async t => {
}
]

await t.throws(client.flush(), 'timeout of 500ms exceeded')
// await t.throwsAsync(() => client.flush(), { message: 'timeout of 500ms exceeded' })
try {
await t.throwsAsync(() => client.flush())
} catch (error) {
console.log('Dafuq:', error)
}
})

test('flush - skip when client is disabled', async t => {
Expand Down Expand Up @@ -422,8 +432,8 @@ test('identify - require a userId or anonymousId', t => {
const client = createClient()
stub(client, 'enqueue')

t.throws(() => client.identify(), 'You must pass a message object.')
t.throws(() => client.identify({}), 'You must pass either an "anonymousId" or a "userId".')
t.throws(() => client.identify(), { message: 'You must pass a message object.' })
t.throws(() => client.identify({}), { message: 'You must pass either an "anonymousId" or a "userId".' })
t.notThrows(() => client.identify({ userId: 'id' }))
t.notThrows(() => client.identify({ anonymousId: 'id' }))
})
Expand All @@ -447,10 +457,10 @@ test('group - require a groupId and either userId or anonymousId', t => {
const client = createClient()
stub(client, 'enqueue')

t.throws(() => client.group(), 'You must pass a message object.')
t.throws(() => client.group({}), 'You must pass either an "anonymousId" or a "userId".')
t.throws(() => client.group({ userId: 'id' }), 'You must pass a "groupId".')
t.throws(() => client.group({ anonymousId: 'id' }), 'You must pass a "groupId".')
t.throws(() => client.group(), { message: 'You must pass a message object.' })
t.throws(() => client.group({}), { message: 'You must pass either an "anonymousId" or a "userId".' })
t.throws(() => client.group({ userId: 'id' }), { message: 'You must pass a "groupId".' })
t.throws(() => client.group({ anonymousId: 'id' }), { message: 'You must pass a "groupId".' })
t.notThrows(() => {
client.group({
groupId: 'id',
Expand Down Expand Up @@ -485,10 +495,10 @@ test('track - require event and either userId or anonymousId', t => {
const client = createClient()
stub(client, 'enqueue')

t.throws(() => client.track(), 'You must pass a message object.')
t.throws(() => client.track({}), 'You must pass either an "anonymousId" or a "userId".')
t.throws(() => client.track({ userId: 'id' }), 'You must pass an "event".')
t.throws(() => client.track({ anonymousId: 'id' }), 'You must pass an "event".')
t.throws(() => client.track(), { message: 'You must pass a message object.' })
t.throws(() => client.track({}), { message: 'You must pass either an "anonymousId" or a "userId".' })
t.throws(() => client.track({ userId: 'id' }), { message: 'You must pass an "event".' })
t.throws(() => client.track({ anonymousId: 'id' }), { message: 'You must pass an "event".' })
t.notThrows(() => {
client.track({
userId: 'id',
Expand Down Expand Up @@ -519,8 +529,8 @@ test('page - require either userId or anonymousId', t => {
const client = createClient()
stub(client, 'enqueue')

t.throws(() => client.page(), 'You must pass a message object.')
t.throws(() => client.page({}), 'You must pass either an "anonymousId" or a "userId".')
t.throws(() => client.page(), { message: 'You must pass a message object.' })
t.throws(() => client.page({}), { message: 'You must pass either an "anonymousId" or a "userId".' })
t.notThrows(() => client.page({ userId: 'id' }))
t.notThrows(() => client.page({ anonymousId: 'id' }))
})
Expand All @@ -540,8 +550,8 @@ test('screen - require either userId or anonymousId', t => {
const client = createClient()
stub(client, 'enqueue')

t.throws(() => client.screen(), 'You must pass a message object.')
t.throws(() => client.screen({}), 'You must pass either an "anonymousId" or a "userId".')
t.throws(() => client.screen(), { message: 'You must pass a message object.' })
t.throws(() => client.screen({}), { message: 'You must pass either an "anonymousId" or a "userId".' })
t.notThrows(() => client.screen({ userId: 'id' }))
t.notThrows(() => client.screen({ anonymousId: 'id' }))
})
Expand All @@ -565,9 +575,9 @@ test('alias - require previousId and userId', t => {
const client = createClient()
stub(client, 'enqueue')

t.throws(() => client.alias(), 'You must pass a message object.')
t.throws(() => client.alias({}), 'You must pass a "userId".')
t.throws(() => client.alias({ userId: 'id' }), 'You must pass a "previousId".')
t.throws(() => client.alias(), { message: 'You must pass a message object.' })
t.throws(() => client.alias({}), { message: 'You must pass a "userId".' })
t.throws(() => client.alias({ userId: 'id' }), { message: 'You must pass a "previousId".' })
t.notThrows(() => {
client.alias({
userId: 'id',
Expand Down Expand Up @@ -607,7 +617,7 @@ test('dont allow messages > 32kb', t => {

t.throws(() => {
client.track(event, noop)
})
}, { message: 'Your message must be < 32kb.' })
})

test('ensure that failed requests are retried', async t => {
Expand All @@ -621,7 +631,9 @@ test('ensure that failed requests are retried', async t => {
}
]

await t.notThrows(client.flush())
await t.notThrowsAsync(async () => {
await client.flush()
})
})

test('ensure that failed requests are not retried forever', async t => {
Expand All @@ -635,7 +647,8 @@ test('ensure that failed requests are not retried forever', async t => {
}
]

await t.throws(client.flush())
// await t.throwsAsync(() => client.flush(), { message: 'Service Unavailable' })
await t.throwsAsync(client.flush())
})

test('ensure we can pass our own axios instance', async t => {
Expand Down