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

Finnp patch 1 - Handle error events correctly #164

Merged
merged 4 commits into from
Jul 26, 2019
Merged

Conversation

naugtur
Copy link
Owner

@naugtur naugtur commented Apr 28, 2017

@finnp I rebased your PR onto current v3 work
I'm making minor changes and I want this to have tests too.

You should find an invite to be a collaborator in your email, which lets you work on this branch directly.

finnp and others added 3 commits March 23, 2017 14:44
The `onerror` and `ontimeout` handler are not calling back with errors but with error events (without any messages). Therefore the `errorFunc` is wrapping them as `Error: [ProgressEvent object]`. 

Here's my quick proposal on how to fix this. Just pass generic error messages to the `errorFunc`, then there will be no case where the error needs to be constructed there.
@naugtur
Copy link
Owner Author

naugtur commented May 2, 2017

@finnp Could you review what I changed on top of your PR and let me know if you have any concerns?

Copy link
Collaborator

@finnp finnp left a comment

Choose a reason for hiding this comment

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

LGTM

@nicksp
Copy link

nicksp commented Jul 24, 2019

@naugtur Hey, we're affected by a similar problem with unhandled error on a project, so wondering when do you plan to ship changes into master? We found suggested changes quite helpful and would like to see them merged into the main branch 😏

For the time being, we forked the repo and did local changes, so we can reference package.json to a fork instead.

@naugtur
Copy link
Owner Author

naugtur commented Jul 24, 2019

I was going to release a new major version for quite a while now, but ran out of time and contributors. If you could help, it becomes more likely.
Now that request package is deprecated I need to rethink which changes to put in the new and likely last bigger release of xhr.

@nicksp
Copy link

nicksp commented Jul 25, 2019

@naugtur What kind of help do you expect? Tell me how I can help and I'll try to allocate time.

@naugtur
Copy link
Owner Author

naugtur commented Jul 26, 2019

@nicksp I don't have any expectations. I'm just a humble developer that happens to be involved in this project :)

If you want to help, the absolute first step is this:
#81

It'd allow improvements to tests and make it more certain that changes don't break anything.

With request being deprecated I'm not sure if dropping old IE support is such a great idea, but everything marked as v3 milestone is tracked in here and work has been started.
See more here: #136

With 2 people involved I was hoping to get the v3 done and published in 2017, but we didn't end up getting to it.
I'd like to finish off and ship the last big version and then mark this library as done - no more features, low maintenance (same as in the last 2 years)

@naugtur naugtur merged commit bb0ee82 into drop-IE8-9 Jul 26, 2019
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.

3 participants