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

Fixed abort/error/loadend event firing, statusCode on error, exceptions #6

Merged
merged 2 commits into from
Jul 31, 2019
Merged
Show file tree
Hide file tree
Changes from all 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
72 changes: 44 additions & 28 deletions lib/XMLHttpRequest.js
Original file line number Diff line number Diff line change
Expand Up @@ -102,6 +102,7 @@ function XMLHttpRequest(opts) {
var sendFlag = false;
// Error flag, used when errors occur or abort is called
var errorFlag = false;
var abortedFlag = false;

// Event listeners
var listeners = {};
Expand Down Expand Up @@ -172,10 +173,11 @@ function XMLHttpRequest(opts) {
this.open = function(method, url, async, user, password) {
this.abort();
errorFlag = false;
abortedFlag = false;

// Check for valid request method
if (!isAllowedHttpMethod(method)) {
throw "SecurityError: Request method not allowed";
throw new Error("SecurityError: Request method not allowed");
}

settings = {
Expand Down Expand Up @@ -208,16 +210,14 @@ function XMLHttpRequest(opts) {
*/
this.setRequestHeader = function(header, value) {
if (this.readyState != this.OPENED) {
throw "INVALID_STATE_ERR: setRequestHeader can only be called when state is OPEN";
return false;
throw new Error("INVALID_STATE_ERR: setRequestHeader can only be called when state is OPEN");
}
if (!isAllowedHttpHeader(header)) {
console.warn('Refused to set unsafe header "' + header + '"');
return false;
}
if (sendFlag) {
throw "INVALID_STATE_ERR: send flag is true";
return false;
throw new Error("INVALID_STATE_ERR: send flag is true");
}
headers[header] = value;
return true;
Expand Down Expand Up @@ -283,11 +283,11 @@ function XMLHttpRequest(opts) {
*/
this.send = function(data) {
if (this.readyState != this.OPENED) {
throw "INVALID_STATE_ERR: connection must be opened before send() is called";
throw new Error("INVALID_STATE_ERR: connection must be opened before send() is called");
}

if (sendFlag) {
throw "INVALID_STATE_ERR: send has already been called";
throw new Error("INVALID_STATE_ERR: send has already been called");
}

var ssl = false, local = false;
Expand All @@ -312,13 +312,13 @@ function XMLHttpRequest(opts) {
break;

default:
throw "Protocol not supported.";
throw new Error("Protocol not supported.");
}

// Load files off the local filesystem (file://)
if (local) {
if (settings.method !== "GET") {
throw "XMLHttpRequest: Only GET method is supported";
throw new Error("XMLHttpRequest: Only GET method is supported");
}

if (settings.async) {
Expand Down Expand Up @@ -402,7 +402,6 @@ function XMLHttpRequest(opts) {

// Reset error flag
errorFlag = false;

// Handle async requests
if (settings.async) {
// Use the proper protocol
Expand Down Expand Up @@ -545,7 +544,7 @@ function XMLHttpRequest(opts) {
if (self.responseText.match(/^NODE-XMLHTTPREQUEST-ERROR:/)) {
// If the file returned an error, handle it
var errorObj = self.responseText.replace(/^NODE-XMLHTTPREQUEST-ERROR:/, "");
self.handleError(errorObj);
self.handleError(errorObj, 503);
} else {
// If the file returned okay, parse its data and move to the DONE state
self.status = self.responseText.replace(/^NODE-XMLHTTPREQUEST-STATUS:([0-9]*),.*/, "$1");
Expand All @@ -557,9 +556,10 @@ function XMLHttpRequest(opts) {

/**
* Called when an error is encountered to deal with it.
* @param status {number} HTTP status code to use rather than the default (0) for XHR errors.
*/
this.handleError = function(error) {
this.status = 503;
this.handleError = function(error, status) {
this.status = +status || 0;
Copy link
Owner

Choose a reason for hiding this comment

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

Why is there a + in front of status?

this.statusText = error;
this.responseText = error.stack;
errorFlag = true;
Expand All @@ -579,8 +579,7 @@ function XMLHttpRequest(opts) {
this.responseText = "";
this.responseXML = "";

errorFlag = true;

errorFlag = abortedFlag = true
if (this.readyState !== this.UNSENT
&& (this.readyState !== this.OPENED || sendFlag)
&& this.readyState !== this.DONE) {
Expand Down Expand Up @@ -619,11 +618,17 @@ function XMLHttpRequest(opts) {
*/
this.dispatchEvent = function(event) {
if (typeof self["on" + event] === "function") {
self["on" + event]();
if (this.readyState === this.DONE)
setImmediate(function() { self["on" + event]() })
else
self["on" + event]()
}
if (event in listeners) {
for (var i = 0, len = listeners[event].length; i < len; i++) {
listeners[event][i].call(self);
for (let i = 0, len = listeners[event].length; i < len; i++) {
if (this.readyState === this.DONE)
setImmediate(function() { listeners[event][i].call(self) })
else
listeners[event][i].call(self)
}
}
};
Expand All @@ -634,18 +639,29 @@ function XMLHttpRequest(opts) {
* @param int state New state
*/
var setState = function(state) {
if (self.readyState !== state) {
self.readyState = state;
if ((self.readyState === state) || (self.readyState === self.UNSENT && abortedFlag))
return

if (settings.async || self.readyState < self.OPENED || self.readyState === self.DONE) {
self.dispatchEvent("readystatechange");
}
self.readyState = state;

if (self.readyState === self.DONE && !errorFlag) {
self.dispatchEvent("load");
// @TODO figure out InspectorInstrumentation::didLoadXHR(cookie)
self.dispatchEvent("loadend");
}
if (settings.async || self.readyState < self.OPENED || self.readyState === self.DONE) {
self.dispatchEvent("readystatechange");
}

if (self.readyState === self.DONE) {
let fire

if (abortedFlag)
fire = "abort"
else if (errorFlag)
fire = "error"
else
fire = "load"

self.dispatchEvent(fire)

// @TODO figure out InspectorInstrumentation::didLoadXHR(cookie)
self.dispatchEvent("loadend");
}
};
};
2 changes: 1 addition & 1 deletion package.json
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
{
"name": "xmlhttprequest-ssl",
"description": "XMLHttpRequest for Node",
"version": "1.5.5",
"version": "1.5.6",
Copy link
Owner

Choose a reason for hiding this comment

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

In my opinion your changes will at least warrant a minor version bump. Technically it should even be a major, but I doubt anyone's really relying on the specific error type or 503 status code being returned. If you could change it to be 1.6.0 that'd be awesome!

"author": {
"name": "Michael de Wit"
},
Expand Down