-
Notifications
You must be signed in to change notification settings - Fork 0
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
feat: Add possibility to record calls #111
Conversation
index.js
Outdated
recordingsContext.deleteBodyAttributes.forEach((attr) => { | ||
try { | ||
console.log('Deleting attribute', attr); | ||
eval(`delete obfuscatedBody.${attr}`); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
function deleteNestedProperty(obj, path) {
const keys = path.split('.');
const lastKey = keys.pop();
const lastObj = keys.reduce((acc, key) => acc && acc[key], obj);
if (lastObj && lastKey in lastObj) {
delete lastObj[lastKey];
}
}
deleteNestedProperty(obfuscatedBody, attr);
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
👍
index.js
Outdated
method: req.method, | ||
headers: req.headers, | ||
}; | ||
const crypto = require('crypto'); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
move to top
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
👍
index.js
Outdated
@@ -168,7 +194,71 @@ app.all('*', (req, res) => { | |||
} | |||
} | |||
|
|||
const errorMessage = `Request ${req.url} didn't match any registered route.`; | |||
const obfuscatedBody = JSON.parse(JSON.stringify(req.body)); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
maybe obfuscatedReqBody
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
obfuscatedReqBodyForHash
index.js
Outdated
return; | ||
} | ||
|
||
const responseFromHash = Object.values(recordings).find((rec) => rec[hash]); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
why not recordings[recordingsContext.namespace][hash]
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good catch... this was before I always had namespace context
index.js
Outdated
const hash = crypto.createHash('sha512').update(JSON.stringify(dataToHash)).digest('hex'); | ||
if (recordingsContext.active) { | ||
try { | ||
const host = req.url.split('/')[1]; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
const [_, host, ...routeParts] = req.url.split('/');
const route = `/${routeParts.join('/')}`;
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
👍
index.js
Outdated
const proxyRes = await fetch(targetUrl, { | ||
method: req.method, | ||
headers: { ...req.headers, 'Access-Control-Allow-Origin': '*' }, | ||
...(req.method !== 'GET' && req.method !== 'HEAD' ? { body: JSON.stringify(req.body) } : {}), |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
&& req.body
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
👍
console.log('No attribute to delete: ', attr); | ||
} | ||
}); | ||
const dataToHash = { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
move to own function getHashForRequest
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
👍
index.js
Outdated
@@ -102,6 +110,25 @@ route.post('/reset/calls', (_req, res) => { | |||
res.sendStatus(204); | |||
}); | |||
|
|||
route.post('/record', (req, res) => { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
proposal. just one route /recording (without s, referring to the functionality) with two methods:
POST /recording updates the full recording context, including recordings (that you'd set separately with /load-recordings in your current approach)
GET /recording returns the full recording context, including recordings
index.js
Outdated
recordings[hash].push({ | ||
body: typeof body === 'string' ? body : JSON.stringify(body), | ||
status, | ||
hashData: { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think that to call it "request" would be more informative. Would make data in recordings folder more legible.
index.js
Outdated
recordings[hash] = []; | ||
} | ||
recordings[hash].push({ | ||
body: typeof body === 'string' ? body : JSON.stringify(body), |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
maybe don't stringify
🎉 This PR is included in version 2.17.0 🎉 The release is available on:
Your semantic-release bot 📦🚀 |
No description provided.