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

feat: Add error event and improve error handling #185

Merged
merged 3 commits into from
Feb 20, 2019
Merged
Show file tree
Hide file tree
Changes from 2 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
18 changes: 18 additions & 0 deletions docs/server/events-and-middleware.md
Original file line number Diff line number Diff line change
Expand Up @@ -126,6 +126,24 @@ server.any().on('beforeReplay', (req, recording) => {
});
```

### error

Fires when any error gets emitted during the request life-cycle.

| Param | Type | Description |
| ----- | ------------------------- | -------------------- |
| req | [Request](server/request) | The request instance |
| error | Error | The error |

**Example**

```js
server.any().on('error', (req, error) => {
console.error(error);
process.exit(1);
});
```

## Middleware

Middleware can be added via the `.any()` method.
Expand Down
11 changes: 9 additions & 2 deletions packages/@pollyjs/adapter-node-http/src/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -198,9 +198,16 @@ export default class HttpAdapter extends Adapter {
};
}

async respondToRequest(pollyRequest) {
const { statusCode, body, headers } = pollyRequest.response;
async respondToRequest(pollyRequest, error) {
const { req, respond } = pollyRequest.requestArguments;

if (error) {
respond(error);

return;
}

const { statusCode, body, headers } = pollyRequest.response;
const chunks = this.getChunksFromBody(body, headers);
const stream = new Readable();

Expand Down
Original file line number Diff line number Diff line change
@@ -1,8 +1,7 @@
export default function getResponseFromRequest(req, data) {
return new Promise(resolve => {
req.on('response', res => {
resolve(res);
});
return new Promise((resolve, reject) => {
req.once('response', resolve);
req.once('error', reject);

req.end(data);
});
Expand Down
29 changes: 10 additions & 19 deletions packages/@pollyjs/adapter-puppeteer/src/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -156,18 +156,6 @@ export default class PuppeteerAdapter extends Adapter {
this._requestsMapping.pollyRequests.set(request, pollyRequest);
}

/**
* Abort the request on failure.
*
* @override
*/
async onRequestFailed(pollyRequest) {
const { request } = pollyRequest.requestArguments;

await request.abort();
await super.onRequestFailed(...arguments);
}

async passthroughRequest(pollyRequest) {
const { page } = this.options;
const { id, order, url, method, headers, body } = pollyRequest;
Expand Down Expand Up @@ -203,22 +191,25 @@ export default class PuppeteerAdapter extends Adapter {
body: await response.text()
};
} catch (error) {
console.error(error);
throw error;
} finally {
this[PASSTHROUGH_PROMISES].delete(requestId);
}
}

respondToRequest(pollyRequest) {
async respondToRequest(pollyRequest, error) {
const { request } = pollyRequest.requestArguments;
const { response } = pollyRequest;

request.respond({
status: response.statusCode,
headers: response.headers,
body: response.body
});
if (error) {
await request.abort();
offirgolan marked this conversation as resolved.
Show resolved Hide resolved
} else {
await request.respond({
status: response.statusCode,
headers: response.headers,
body: response.body
});
}
}

_callListenersWith(methodName, target) {
Expand Down
11 changes: 8 additions & 3 deletions packages/@pollyjs/adapter-xhr/src/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -44,11 +44,16 @@ export default class XHRAdapter extends Adapter {
this.xhr.restore();
}

respondToRequest(pollyRequest) {
respondToRequest(pollyRequest, error) {
const { xhr } = pollyRequest.requestArguments;
const { response } = pollyRequest;

xhr.respond(response.statusCode, response.headers, response.body);
if (error) {
xhr.error();
} else {
const { response } = pollyRequest;

xhr.respond(response.statusCode, response.headers, response.body);
}
}

async passthroughRequest(pollyRequest) {
Expand Down
2 changes: 1 addition & 1 deletion packages/@pollyjs/adapter-xhr/tests/utils/xhr-request.js
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@ export default function request(url, obj = {}) {
xhr.status === 204 && xhr.responseText === '' ? null : xhr.responseText;

return new Response(responseBody, {
status: xhr.status,
status: xhr.status || 500,
Copy link
Contributor

Choose a reason for hiding this comment

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

I realize this is just pertaining to a test util but can the fallback mask a future bug in our test suite?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

This will only happen when status is 0 which occurs when we do xhr.error(). Do you have a better way around this? Response wont accept a status code of 0

statusText: xhr.statusText,
headers: serializeResponseHeaders(xhr.getAllResponseHeaders())
});
Expand Down
7 changes: 6 additions & 1 deletion packages/@pollyjs/adapter/src/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -300,7 +300,12 @@ export default class Adapter {
* @param {PollyRequest} pollyRequest
* @param {Error} error
*/
onRequestFailed(pollyRequest, error) {
async onRequestFailed(pollyRequest, error) {
error =
error || new Error('[Polly] Request failed due to an unknown error.');

await pollyRequest._emit('error', error);
await this.respondToRequest(pollyRequest, error);
Copy link
Collaborator Author

@offirgolan offirgolan Feb 19, 2019

Choose a reason for hiding this comment

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

Call respondToRequest with an error if the request failed so that the adapters can gracefully handle it.

pollyRequest.promise.reject(error);
}
}
10 changes: 9 additions & 1 deletion packages/@pollyjs/adapter/src/utils/stringify-request.js
Original file line number Diff line number Diff line change
@@ -1,4 +1,12 @@
export default function stringifyRequest(req, ...args) {
const config = { ...req.config };

// Remove all adapter & persister config options as they can cause a circular
// structure to the final JSON
['adapter', 'adapterOptions', 'persister', 'persisterOptions'].forEach(
Copy link
Contributor

Choose a reason for hiding this comment

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

Is this a fix for separate bug?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

This is a fix for a separate bug that I found when tackling this.

k => delete config[k]
);

return JSON.stringify(
{
url: req.url,
Expand All @@ -9,7 +17,7 @@ export default function stringifyRequest(req, ...args) {
id: req.id,
order: req.order,
identifiers: req.identifiers,
config: req.config
config
},
...args
);
Expand Down
23 changes: 0 additions & 23 deletions packages/@pollyjs/core/src/-private/logger.js
Original file line number Diff line number Diff line change
Expand Up @@ -13,10 +13,6 @@ export default class Logger {
this.groupName = null;
}

get enabled() {
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Removing dead/unused code.

return this.polly.config.logging;
}

connect() {
this._middleware = this.polly.server
.any()
Expand All @@ -28,13 +24,6 @@ export default class Logger {
this._middleware.off('response');
}

console(method, ...args) {
if (this.enabled) {
this.groupStart(this.polly.recordingName);
console[method].apply(console, args);
}
}

groupStart(groupName) {
// If the provided groupName is different, end the current group so a new one
// can be started.
Expand Down Expand Up @@ -68,16 +57,4 @@ export default class Logger {
);
}
}

log() {
this.console('log', ...arguments);
}

warn() {
this.console('warn', ...arguments);
}

error() {
this.console('error', ...arguments);
}
}
1 change: 1 addition & 0 deletions packages/@pollyjs/core/src/server/handler.js
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ export default class Handler extends Map {
this.set('config', {});
this._eventEmitter = new EventEmitter({
eventNames: [
'error',
'request',
'beforeReplay',
'beforePersist',
Expand Down
20 changes: 20 additions & 0 deletions tests/integration/adapter-tests.js
Original file line number Diff line number Diff line change
Expand Up @@ -126,6 +126,26 @@ export default function adapterTests() {
]);
});

it('should emit an error event', async function() {
const { server } = this.polly;
let error;

this.polly.configure({ recordIfMissing: false });

server.get(this.recordUrl()).on('error', (req, err) => (error = err));

try {
await this.fetchRecord();
} catch (e) {
/* noop */
}

expect(error).to.exist;
expect(error.message).to.match(
/Recording for the following request is not found/
);
});

it('should handle a compressed response', async function() {
const res = await this.relativeFetch('/compress', {
method: 'POST',
Expand Down