feat($http): add xhr statusText to completeRequest callback

[jimlyndon]

Makes xhr status text accessible is $http success/error callback.
See www.w3.org/TR/XMLHttpRequest/#dom-xmlhttprequest-statustext

Closes #2335

[petebacondarwin]

@jimlyndon : Thanks for this PR. In its current form, this feature would break lots of code that is already in production. It may be feasible to only add status text as the last parameter, since it would not be breaking.

But I suspect that this is not going to be a useful enough addition to the core project. It is perfectly possible to access the status text if you need it by providing handlers through .then(), rather than using success() and error().

[jimlyndon]

Ah, yes, I should add statusText to the end of the parameter list in the promise success/error as that interface would break for current users. Probably should write a test to check that as well.

But I suspect that this is not going to be a useful enough addition to the core project. It is perfectly possible to access the status text if you need it by providing handlers through .then(), rather than using success() and error().

Can you explain a little more. As far as I can tell, HttpBackend doesn't expose statusText on the XMLHttpRequest object. Providing a handler would be too late I believe.

Thanks @petebacondarwin

[petebacondarwin]

@jimlyndon - OK, looking through more carefully now I see that this is not available. We'll need the param moving to the end of the param list and we'll also need the core team to agree to adding this.

Personally, I would just rely on the status code and create a user friendy message string mapping in my application.

BTW, are statusTexts language dependent? I.E. does the browser provide different strings based on its language setting? If so, what would you do about the hard-coded statusTexts, like "No Content"?

[jimlyndon]

@petebacondarwin,

Personally, I would just rely on the status code and create a user friendy message string mapping in my application.

I agree, though it's not always an option when building clients against existing APIs.

BTW, are statusTexts language dependent? I.E. does the browser provide different strings based on its language setting? If so, what would you do about the hard-coded statusTexts, like "No Content"?

Language settings in the browser won't affect the response, and if you want to manage something like localisation you have to do that yourself (server or client side). The browser just passes on the value, whatever the case, just like it does the response and (normally) the status code (with a few known exceptions)

I'll reorder the parameters since callbacks depend on the order and I'll add some new tests to cover that exact scenario (which would be a good test to have even without including the feature). I agree with making it as non-invasive as possible.

And I completely understand wanting to keep the code base small and not add every little feature, Though this is a pretty standard w3c api that's been consistently available, and should be a trivial addition (at least after I make your change request :).

[jimlyndon]

Updated. I also followed your initial suggestion and simplified the $http changes (statusText accessible through 'then') leaving success/error callbacks as they are, making this PR less invasive. Let me know what you think.

[petebacondarwin]

I'll take a look tomorrow.

On 20 May 2013 13:24, Jim Lyndon notifications@github.com wrote:

Updated. I also followed your initial suggestion and simplified the $http
changes (statusText accessible through 'then') leaving success/error
callbacks as they are, making this PR less invasive. Let me know what you
think.

—
Reply to this email directly or view it on GitHubhttps://github.com//pull/2665#issuecomment-18144650
.

[jimlyndon]

Any news on this PR?

[protoze]

Is there any chance of this making it into the trunk?

[artch]

+1 on this PR.

[petebacondarwin]

PR Checklist (Minor Feature)

• Contributor signed CLA now or in the past (if you just signed, leave a comment here with your real name for cross reference)
• Feature improves existing core functionality
• API is compatible with existing Angular apis and relevant standards (if applicable)
• PR doesn't contain a breaking change
• PR contains unit tests
• PR contains e2e tests (if suitable)
• PR contains documentation update
• PR passes all tests on Travis (sanity)
• PR passes all tests on ci.angularjs.org (cross-browser compatibility)
• PR is rebased against recent master
• PR is squashed into one commit per logical change
• PR's commit messages are descriptive and allows us to autogenerate release notes (required commit message format)
• All changes requested in review have been implemented

[jimlyndon]

CLA is signed with "Jim Lyndon"
I'll try to get to the rest over the next few days.

[jimlyndon]

Rebased and squashed against recent master. Travis pooped out with that NgClosureRunner dependency err, which appears to happen occasionally. Thanks

[mikeobrien]

Any updates on this? We are dealing with the same thing, need to display the status text in the UI.

[jgrund]

Why not expose the raw xhr object? It could be helpful to have access to other properties that live there.

[protoze]

This isn't in 1.2.1? arrrgghhhh....

[uicestone]

Just waiting for this, any update? Should I wait for 1.2.127?

[lennybacon]

Wow 9 month and nobody seem to care

[caitp]

I don't think it's worth worrying about adding statusText to the success/error callbacks, but I don't see any reason why we couldn't get this un-bitrotten and merge it. I was just investigating and most of it, apart from changes to Mocks, could be added in very few lines of code in httpBackend.js and http.js, and create a framework for further extensions in case other objects need to be exposed in the future.

@jimlyndon do you want to fix this PR up a bit? otherwise I could put one together --- edit err, I didn't mean @lennybacon, sorry about that ;)

[benbenwilde]

+1 Let's get this thing going!
This is very important for any HTTP API that sends down error messages or exception names in the reason phrase.
A status code to error message dictionary is NOT sufficient.
This is only needed for .then() callbacks, and it needs to do nothing more than add a field to the response object.

[jimlyndon]

Hey friends, yes this PR got the go ahead however there was a travis build issue and I couldn't get any eyeballs from the core team. I would love this to go in, but I'm absolutely slammed with work and real life at the moment (moving, wedding planning, you know how that goes). If someone wants to rebase what I have here, it's a few changes to http and backend. Cheers!

[caitp]

Sure, I'll fix the bitrot