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

[BUG] FedoraRecord.get() allows non 200 responses to fail silently #2

Open
markpbaggett opened this issue May 14, 2020 · 0 comments
Open
Labels
bug Something isn't working

Comments

@markpbaggett
Copy link
Owner

Describe the bug
FedoraRecord.get() returns an empty string if the response is not "200 OK":

method get(this: FedoraRecord): string {. base .} =
  let response = this.client.request(this.uri, httpMethod = HttpGet)
  if response.status == "200 OK":
    response.body
  else:
    ""

Because of this, we are assuming that other things that use FedoraRequest.get() expect an empty string to equate to a 500, 401, or a 403. While this might be perfectly fine for some uses, it sure is not for all and we aren't doing anything to log potential problems.

Expected behavior
Instead of returning an empty string, we should:

  1. Minimally, log the fail.
  2. Sleep and retry the request.

Doing the latter, should have effects on the FedoraRequest type and not solely this method.

Screenshots

method get(this: FedoraRecord): string {. base .} =
  let response = this.client.request(this.uri, httpMethod = HttpGet)
  if response.status == "200 OK":
    response.body
  else:
    ""

Additional context
It should be noted that all "200 OK" related methods on FedoraRequest should be reviewed, but we should do those in separate issues as the approach may be slightly different.

@markpbaggett markpbaggett added the bug Something isn't working label May 14, 2020
mathewjordan pushed a commit to mathewjordan/moldybread that referenced this issue May 18, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

1 participant