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

Cannot read response object from error when http status is 404 #261

Closed
dpakach opened this issue May 11, 2021 · 7 comments
Closed

Cannot read response object from error when http status is 404 #261

dpakach opened this issue May 11, 2021 · 7 comments

Comments

@dpakach
Copy link
Contributor

dpakach commented May 11, 2021

When making a dav request, if we get 4xx status code, then we cannot read the response object from the error.

eg.

const { createClient } = require("webdav");

const client = createClient(
    "http://<dav-server>",
    {
        username: "admin",
        password: "admin"
    }
);

// Get directory contents for a folder that doesn't exists
const directoryItems = client.getDirectoryContents("/non-existing-file");

directoryItems.then(items => {
  console.log(items)
}).catch(err => {
  console.log(err.response.data)
}) 

In version 4.1.0 we can read the body of the actual response

node index.js
<?xml version="1.0" encoding="utf-8"?>
<d:error xmlns:d="DAV:" xmlns:s="http://sabredav.org/ns">
  <s:exception>Sabre\DAV\Exception\NotFound</s:exception>
  <s:message>File with name non-existing-file could not be located</s:message>
</d:error>

but with the latest version 4.4.0, the response object is not returned in the error

 node index.js
(node:92563) UnhandledPromiseRejectionWarning: TypeError: Cannot read property 'data' of undefined
    at directoryItems.then.catch.err (/home/dipakacharya/Documents/playground/davtest/index.js:17:28)
    at process._tickCallback (internal/process/next_tick.js:68:7)
(node:92563) UnhandledPromiseRejectionWarning: Unhandled promise rejection. This error originated either by throwing inside of an async function without a catch block, or by rejecting a promise which was not handled with .catch(). (rejection id: 1)
(node:92563) [DEP0018] DeprecationWarning: Unhandled promise rejections are deprecated. In the future, promise rejections that are not handled will terminate the Node.js process with a non-zero exit code. 
@qtarant
Copy link

qtarant commented May 12, 2021

Hello. I have the exact same problem. I am using the owncloud-sdk which depends on this package and it have the same problem. I have tested different releases (4.0 and above) but all of them have the same problem.

@perry-mitchell
Copy link
Owner

Thanks for the report, looking into this now.

@perry-mitchell
Copy link
Owner

I don't see that 4.1.0 would have had response.data on the errors being thrown from this library: v4.1.0...v4.2.0

Can someone point me to the regression that occurred after 4.1.0?

Also, if you check the types, error.response is not a supported property, so I can't see how this would have worked in any 4.x version. I am happy adding such support, however.

@dpakach
Copy link
Contributor Author

dpakach commented May 13, 2021

@perry-mitchell, found that the issue was because the error is not handled by axios for any status codes (https://github.com/perry-mitchell/webdav-client/blob/master/source/request.ts#L52).
When axios generated the error it added all those additional properties but now the handleResponseCode() only added status property to the error.

I've made a simple fix PR that worked for me, please have a look #262
thanks

@qtarant
Copy link

qtarant commented May 13, 2021

I can confirm that the fix proposed for @dpakach in his last comment (#262) worked for me also, now I can get the error message in responses with status >= 400
Thanks a lot, @dpakach

@perry-mitchell
Copy link
Owner

Thanks @dpakach - I left a review on the MR. If we agree I'll happily merge it this week.

@dpakach
Copy link
Contributor Author

dpakach commented May 21, 2021

@perry-mitchell, I've fixed the PR according to your review, please have a look

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

No branches or pull requests

3 participants