szmarczak / http-timer

🕐 Performance timings for HTTP requests
MIT License
192 stars 18 forks source link

total is not set if there is an error #3

Closed jstewmon closed 5 years ago

jstewmon commented 6 years ago

Error events are not listened for, so if the request or response terminates due to an error, total will not be set.

jstewmon commented 5 years ago

I think you also need to listen for response's error event. For example, if there's an error reading the response body.

szmarczak commented 5 years ago

Fixed in 9fac585ca5863be0fec278dd2d772fd39f9e32a6

jstewmon commented 5 years ago

Was there a reason you wanted to patch emit? prependListener allows you to move to the front of the line if other listeners have already been attached.

szmarczak commented 5 years ago

Was there a reason you wanted to patch emit?

Yup. The implementation you're talking about is:

    const handleError = origin => {
        const errorHandler = error => {
            timings.error = Date.now();
            timings.phases.total = timings.error - timings.start;

            origin.off('newListener', newListenerHandler); // eslint-disable-line no-use-before-define

            if (origin.listenerCount('error') === 0) {
                throw error;
            }
        };

        const newListenerHandler = (event, handler) => {
            if (event === 'error' && handler !== errorHandler) {
                origin.off('error', errorHandler);

                // Make sure our handler gets called first
                process.nextTick(() => origin.prependOnceListener('error', errorHandler));
            }
        };

        origin.on('newListener', newListenerHandler);
        origin.once('error', errorHandler);
    };

It's a bit complicated, isn't it? If you wanna send a PR, feel free :) I'm happy to work on this.

The current solution is much shorter and I know for sure it works :P

szmarczak commented 5 years ago

Pardon my very late response, but the approach I had shown was incorrect. I've updated the post above. Please take a look at it! :)