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

check_opencpu_response() should check for content type of API response #300

Closed
Tracked by #297
PietrH opened this issue Sep 12, 2024 · 2 comments
Closed
Tracked by #297
Assignees
Labels
actionable Can be implemented API
Milestone

Comments

@PietrH
Copy link
Member

PietrH commented Sep 12, 2024

Currently check_opencpu_response() only checks for the HTTP status code, but it could also check for the content type.

During #297 , I encountered a case where the content type was binary causing extract_temp_key() to return NA without an error.

Either of these functions should have thrown an error here instead.

The API response was binary because I was passing unsupported arguments to etnservice via the request body

@PietrH PietrH self-assigned this Sep 12, 2024
@PietrH PietrH added actionable Can be implemented API labels Sep 12, 2024
@PietrH PietrH mentioned this issue Sep 12, 2024
16 tasks
@PietrH PietrH added this to the v3.0.0 milestone Sep 12, 2024
@PietrH
Copy link
Member Author

PietrH commented Sep 16, 2024

Both check_opencpu_response() and forward_to_api() (via check_opencpu_response() return the R error as they should and stop operation...

So while I can replicate the OpenCPU binary response with:

httr::POST(
    url = "https://opencpu.lifewatch.be/library/etnservice/R/write_dwc/",
    body = list(credentials = get_credentials(),
                animal_project_code = "2014_demer",
                directory = "."),
    encode = "json"
  )

I can't actually replicate the no error behaviour.

@PietrH
Copy link
Member Author

PietrH commented Sep 16, 2024

Was I getting a different error because of the 403 I was getting? (because functions needed to start with connect_, get_ or list_, now also write_?

But even then the function should have returned an error with the message:

Error: API request failed: Client error: (403) Forbidden

@PietrH PietrH closed this as not planned Won't fix, can't repro, duplicate, stale Sep 16, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
actionable Can be implemented API
Projects
None yet
Development

No branches or pull requests

1 participant