Skip to content

[WIP] Better error message - #122

Merged
boshek merged 8 commits into
masterfrom
error-message
Oct 30, 2019
Merged

[WIP] Better error message#122
boshek merged 8 commits into
masterfrom
error-message

Conversation

@boshek

@boshek boshek commented Oct 29, 2019

Copy link
Copy Markdown
Collaborator

Don't merge until this badge reads green: CRAN status

This PR centralizes the http code error message, adds it to geodata requests and provides a better error message. This should close #121. Here is the current implementation:

> bcdc_get_data(record = 'cff7b8f7-6897-444f-8c53-4bb93c7e9f8b', resource = 'b66b136e-433b-474e-a33c-3c31fd7e0c02')
Error: The BC data catalogue is currently unable to process this request
http status code: 400
> DMK_harv_authority <- bcdc_query_geodata('cff7b8f7-6897-444f-8c53-4bb93c7e9f8b') %>%
 filter(GEOGRAPHIC_DISTRICT_CODE == "DMK") %>%
 collect()
Error: The BC data catalogue is currently unable to process this request
http status code: 400

@boshek boshek mentioned this pull request Oct 29, 2019
Open
@boshek
boshek requested a review from ateucher October 29, 2019 19:10
@boshek boshek added the bug Something isn't working label Oct 29, 2019
@boshek boshek added this to the v0.2.0 milestone Oct 29, 2019
@ateucher

ateucher commented Oct 29, 2019

Copy link
Copy Markdown
Collaborator

Looks great! I wonder if it's worth pulling more status info than just the code, could help the user figure out where the problem is originating... something like this:

https://github.com/ropensci/crul/blob/2338b55d15c3d0de9698fb954c11a7bda6d2aeb8/vignettes/how-to-use-crul.Rmd#L108-L115

@boshek

boshek commented Oct 29, 2019

Copy link
Copy Markdown
Collaborator Author

That's a great idea, particularly since we can leave this PR open and add as time permits.

@boshek

boshek commented Oct 30, 2019

Copy link
Copy Markdown
Collaborator Author

An error looks like this now:

> bcdc_query_geodata('cff7b8f7-6897-444f-8c53-4bb93c7e9f8b')
Error: The BC data catalogue is currently unable to process this request
Catalogue request:
  Content-Type: application/x-www-form-urlencoded
  Accept-Encoding: gzip, deflate
  Accept: application/json, text/xml, application/xml, */*
  User-Agent: https://github.com/bcgov/bcdata
Catalogue response:
  status: HTTP/1.1 400 
  date: Wed, 30 Oct 2019 18:13:34 GMT
  server: Apache
  x-frame-options: allow-from (null)
  x-control-flow-delay-ms: 0
  content-type: application/xml
  set-cookie: GS_FLOW_CONTROL=GS_CFLOW_4db1509e:16e1dde59d3:-3ac4
  access-control-allow-origin: (null)
  access-control-allow-credentials: true
  access-control-allow-methods: POST, GET, OPTIONS, HEAD
  access-control-allow-headers: X-Requested-With, Referer, Origin, Content-Type, SOAPAction, Authorization, Accept
  access-control-max-age: 1000
  connection: close
  transfer-encoding: chunked

ateucher
ateucher approved these changes Oct 30, 2019
View reviewed changes

@ateucher ateucher left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Brilliant! Just one small suggestion for code clarity

Comment thread R/utils.R Outdated
files[supported]
}

catalogue_error <- function(catalogue_response){

@ateucher ateucher Oct 30, 2019

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Do you think we should call this catch_catalogue_error() or something else that better conveys what it's doing?

@boshek boshek Oct 30, 2019

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

Great suggestion

Comment thread R/describe-feature.R Outdated
Comment thread R/utils-classes.R Outdated
Comment thread R/utils-classes.R Outdated
Comment thread R/utils-classes.R Outdated
Comment thread R/utils.R Outdated
boshek and others added 5 commits October 30, 2019 11:29
@boshek @ateucher
Update R/utils-classes.R
7317bff 
Co-Authored-By: Andy Teucher <[email protected]>
@boshek @ateucher
Update R/utils-classes.R
9fe9ea3 
Co-Authored-By: Andy Teucher <[email protected]>
@boshek @ateucher
Update R/describe-feature.R
3f8758c 
Co-Authored-By: Andy Teucher <[email protected]>
@boshek @ateucher
Update R/utils-classes.R
4846690 
Co-Authored-By: Andy Teucher <[email protected]>
@boshek @ateucher
Update R/utils.R
613db1e 
Co-Authored-By: Andy Teucher <[email protected]>
@ateucher

ateucher commented Oct 30, 2019

Copy link
Copy Markdown
Collaborator

Ugh, sorry, just realized that all those suggestions mean separate commits. Ugly

@boshek
boshek merged commit baabd86 into master Oct 30, 2019
@boshek
boshek deleted the error-message branch October 30, 2019 18:32
@boshek boshek mentioned this pull request Jan 28, 2020
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@ateucher ateucher ateucher approved these changes

Assignees

No one assigned

Labels

bug Something isn't working

Projects

None yet

Milestone

v0.2.0

Development

Successfully merging this pull request may close these issues.

bcdc_query_geodata error

2 participants

@boshek @ateucher