Skip to content

Conversation

@ochrons
Copy link

@ochrons ochrons commented Feb 21, 2015

As XHR doesn't offer a clear status for indicating timeout, this method provides an easier way to check for timeout. Tried also using ontimeout but it gets called after onreadystatechange making it rather useless in this case.

Change-Id: Ia0d66bd4ca27f2ce0157258bcc885817efe21a2f

Change-Id: Ia0d66bd4ca27f2ce0157258bcc885817efe21a2f
Copy link
Member

Choose a reason for hiding this comment

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

I'd rather this method be called isTimeout.

@sjrd
Copy link
Member

sjrd commented Feb 21, 2015

Is there some spec somewhere backing the assumption about these status and readyState?

@sjrd
Copy link
Member

sjrd commented Feb 21, 2015

Also, wouldn't it be more appropriate to identity timeouts right where the AjaxException is created, and throw a scala.concurrent.TimeoutException instead?

@ochrons
Copy link
Author

ochrons commented Feb 21, 2015

scala.concurrent.TimeoutException would be ok, but breaks backwards compatibility with code that expects to receive an AjaxException.

Couldn't find any explicit spec on how to detect timeout (except with ontimeout), but that combination of status values seemed to be used elsewhere (for example #37)

@ochrons
Copy link
Author

ochrons commented Feb 21, 2015

Tested this on Chrome, Firefox and IE11 and the timeout detection works ok on all of them.

@sjrd
Copy link
Member

sjrd commented Feb 21, 2015

OK, I guess that makes sense this way, then. I'd still like it to be called isTimeout, to follow Scala's sort-of conventions.

@ochrons
Copy link
Author

ochrons commented Feb 21, 2015

Since I messed up my fork branching and cannot squash these commits, gonna close this PR and redo it The Right Way

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants