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

OCS error response code for Update/Delete shares have changed #3998

Closed
saw-jan opened this issue Jun 21, 2022 · 7 comments · Fixed by #4000
Closed

OCS error response code for Update/Delete shares have changed #3998

saw-jan opened this issue Jun 21, 2022 · 7 comments · Fixed by #4000
Labels

Comments

@saw-jan
Copy link
Member

saw-jan commented Jun 21, 2022

Describe the bug

The error ocs codes for update and delete shares have changed from 998 to 996 and from 400 to 996 respectively. These changes have caused the test's failure (#3996)

The change seems to appear from commit 09b89f7

But the new changes look GOOD. So, I think the tests should be refactored to the new changes.

Steps to reproduce

  1. As admin, share a folder with user1 with permissions 31
  2. As user1, share a folder with user2 with permissions 31
  3. As user2, share a folder with user3 with permissions 17
  4. As user1, try to update/delete the last share (step 3)

Expected behavior (Previously)

Try to edit share

have 998 ocs error response code

curl -uuser1:1234 -XPUT "https://localhost:9200/ocs/v1.php/apps/files_sharing/api/v1/shares/22c7271a-7bec-4883-a6d3-f7fa9ff965b5" \
-d"permissions=31" -k
<ocs>
  <meta>
    <status>error</status>
    <statuscode>998</statuscode>
    <message>cant find requested share</message>
  </meta>
</ocs>

Try to delete share

have 400 ocs error response code

curl -uuser1:1234 -XDELETE "https://localhost:9200/ocs/v1.php/apps/files_sharing/api/v1/shares/22c7271a-7bec-4883-a6d3-f7fa9ff965b5" -vk
<?xml version="1.0" encoding="UTF-8"?>
<ocs>
  <meta>
    <status>error</status>
    <statuscode>400</statuscode>
    <message>missing shareWith</message>
  </meta>
</ocs>

Actual behavior

Try to edit share

The ocs error response code is 996

curl -uuser1:1234 -XPUT "https://localhost:9200/ocs/v1.php/apps/files_sharing/api/v1/shares/6b2481a7-0e20-45eb-9808-0209629f1f5d" \
-d"permissions=31" -k
<?xml version="1.0" encoding="UTF-8"?>
<ocs>
  <meta>
    <status>error</status>
    <statuscode>996</statuscode>
    <message>grpc update share request failed</message>
  </meta>
</ocs>

Try to delete share

The ocs error response code is 996

curl -uuser1:1234 -XDELETE "https://localhost:9200/ocs/v1.php/apps/files_sharing/api/v1/shares/6b2481a7-0e20-45eb-9808-0209629f1f5d" -k
<?xml version="1.0" encoding="UTF-8"?>
<ocs>
  <meta>
    <status>error</status>
    <statuscode>996</statuscode>
    <message>grpc delete share request failed</message>
  </meta>
</ocs>
@saw-jan
Copy link
Member Author

saw-jan commented Jun 21, 2022

IMO, the new changes look OK.
CC @individual-it @phil-davis

@saw-jan saw-jan removed the Type:Bug label Jun 21, 2022
@butonic
Copy link
Member

butonic commented Jun 21, 2022

hm, no, it seems we broke something:

so the 996 hides the actual error

@butonic
Copy link
Member

butonic commented Jun 21, 2022

since this is resharing related, maybe @kobergj already has a hunch

@kobergj
Copy link
Collaborator

kobergj commented Jun 21, 2022

@butonic I can explain the first error code change. The PUT request returns 996 now because GetShare returns NotFound when using the json share manager but BadRequest when going through cs3 share manager. So we can teach ocs service to understand the BadRequest or adjust the error code in cs3 manager.

I don't understand the DELETE error. missing shareWith is only thrown when the shareWith is missing. It doesn't event talk to the share manager in that case.
@saw-jan Are we sure the error message in the DELETE case is the correct one?

@saw-jan
Copy link
Member Author

saw-jan commented Jun 21, 2022

I don't understand the DELETE error. missing shareWith is only thrown when the shareWith is missing. It doesn't event talk to the share manager in that case.

@kobergj Yeah, the error message for DELETE is missing shareWith but that's with older commit id (commit just before 09b89f7).
With current latest oCIS, the DELETE has grpc delete share request failed message

@saw-jan
Copy link
Member Author

saw-jan commented Jun 21, 2022

hm, no, it seems we broke something:

so the 996 hides the actual error

The error messages look related to the request so I thought that was a correct response, my bad!.

@kobergj
Copy link
Collaborator

kobergj commented Jun 21, 2022

@saw-jan Thanks! We found the issue and are working on a fix currently 👍

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

Successfully merging a pull request may close this issue.

3 participants