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.

Sign up for GitHub

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

Jump to bottom

feat: draft: adds a onMissedRequest hook for seeing missed mocks #2682

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open

feat: draft: adds a onMissedRequest hook for seeing missed mocks #2682

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
8 changes: 8 additions & 0 deletions docs/api/MockAgent.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,14 @@ const agent = new Agent()
const mockAgent = new MockAgent({ agent })
```

### Example - MockAgent instantiation with missed request callback

```js
import { MockAgent } from 'undici'

const mockAgent = new MockAgent({ onMissedRequest: (error) => console.log(`A request wasn't handled: ${error.message}`) })
Copy link
Member

Choose a reason for hiding this comment

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

I like the idea pretty much, but not sure if it should be applied to the Agent. Have you considered the interceptor instead?

In that way, I imagine a usage like:

mockPool.intercept({
  path: '/justatest?hello=there&see=ya',
  method: 'GET'
}).reply(200, { wont: 'match' }, {
  headers: { 'content-type': 'application/json' }
// being response the object passed through the intercept
}).onNotMatched((request, response) => console.log(request, response))

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I think I like this usage instead as well. Any potential downsides to this syntax?

Copy link
Member

Choose a reason for hiding this comment

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

I prefer this actually.

```

## Instance Methods

### `MockAgent.get(origin)`
Expand Down
8 changes: 7 additions & 1 deletion lib/mock/mock-agent.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,8 @@ const {
kNetConnect,
kGetNetConnect,
kOptions,
kFactory
kFactory,
kOnMissedRequest
} = require('./mock-symbols')
const MockClient = require('./mock-client')
const MockPool = require('./mock-pool')
Expand All @@ -35,6 +36,11 @@ class MockAgent extends Dispatcher {
const agent = opts?.agent ? opts.agent : new Agent(opts)
this[kAgent] = agent

if (opts?.onMissedRequest && typeof opts.onMissedRequest !== 'function') {
throw new InvalidArgumentError('Argument opts.onMissedRequest must be a callback')
}

this[kOnMissedRequest] = opts?.onMissedRequest
this[kClients] = agent[kClients]
this[kOptions] = buildMockOptions(opts)
}
Expand Down
3 changes: 2 additions & 1 deletion lib/mock/mock-symbols.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,5 +19,6 @@ module.exports = {
kIsMockActive: Symbol('is mock active'),
kNetConnect: Symbol('net connect'),
kGetNetConnect: Symbol('get net connect'),
kConnected: Symbol('connected')
kConnected: Symbol('connected'),
kOnMissedRequest: Symbol('onMissedRequest')
}
7 changes: 6 additions & 1 deletion lib/mock/mock-utils.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,8 @@ const {
kMockAgent,
kOriginalDispatch,
kOrigin,
kGetNetConnect
kGetNetConnect,
kOnMissedRequest
} = require('./mock-symbols')
const { buildURL, nop } = require('../core/util')
const { STATUS_CODES } = require('node:http')
Expand Down Expand Up @@ -307,6 +308,10 @@ function buildMockDispatch () {
mockDispatch.call(this, opts, handler)
} catch (error) {
if (error instanceof MockNotMatchedError) {
if (agent[kOnMissedRequest]) {
agent[kOnMissedRequest](error)
}

const netConnect = agent[kGetNetConnect]()
if (netConnect === false) {
throw new MockNotMatchedError(`${error.message}: subsequent request to origin ${origin} was not allowed (net.connect disabled)`)
Expand Down
39 changes: 39 additions & 0 deletions test/mock-agent.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@
const { test } = require('tap')
const { createServer } = require('node:http')
const { promisify } = require('node:util')
const { stub } = require('sinon')
const { request, setGlobalDispatcher, MockAgent, Agent } = require('..')
const { getResponse } = require('../lib/mock/mock-utils')
const { kClients, kConnected } = require('../lib/core/symbols')
Expand Down Expand Up @@ -385,6 +386,44 @@ test('MockAgent - basic intercept with request', async (t) => {
})
})

test('MockAgent - basic missed request', async (t) => {
t.plan(5)

const server = createServer((req, res) => {
res.setHeader('content-type', 'text/plain')
res.end('hello from the other side')
})
t.teardown(server.close.bind(server))

await promisify(server.listen.bind(server))(0)

const baseUrl = `http://localhost:${server.address().port}`
const onMissedRequest = stub()
const mockAgent = new MockAgent({ onMissedRequest })

setGlobalDispatcher(mockAgent)
t.teardown(mockAgent.close.bind(mockAgent))
const mockPool = mockAgent.get(baseUrl)

mockPool.intercept({
path: '/justatest?hello=there&see=ya',
method: 'GET'
}).reply(200, { wont: 'match' }, {
headers: { 'content-type': 'application/json' }
})

const { statusCode, headers, body } = await request(`${baseUrl}/justatest?hello=there&another=one`, {
method: 'GET'
})

t.equal(statusCode, 200, 'Status code is 200')
t.equal(headers['content-type'], 'text/plain', 'Content type is text/plain')
const responseBody = await getResponse(body)
t.equal(responseBody, 'hello from the other side', 'Body is from the local server')
t.equal(onMissedRequest.calledOnce, true, 'onMissedRequest calledOnce')
t.ok(onMissedRequest.firstCall.args[0].message.includes('Mock dispatch not matched for path \'/justatest?another=one&hello=there\''), 'Error message matches')
})

test('MockAgent - should support local agents', async (t) => {
t.plan(4)

Expand Down