Skip to content
This repository has been archived by the owner on Feb 12, 2023. It is now read-only.

allow any 2XX status code from an external JSON server #469

Merged
merged 1 commit into from
Nov 19, 2019

Conversation

terencehonles
Copy link
Contributor

When a server responds with another 2XX code, such as 201 the server treats this as an error. This accepts any 2XX response.

I used the number 300 instead of SC_MULTIPLE_CHOICES as that constant's name doesn't signify the start of the non success range.

What has been done to verify that this works as intended?

We'll be deploying this shortly.

Why is this the best possible solution? Were any other approaches considered?

Any 2XX code is a success code.

Are there any risks to merging this code? If so, what are they?

No.

Do we need any specific form for testing your changes? If so, please attach one

No

Does this change require updates to documentation? If so, please file an issue at https://github.com/opendatakit/docs/issues/new and include the link below.

Copy link
Contributor

@ggalmazor ggalmazor left a comment

Choose a reason for hiding this comment

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

Thanks, @terencehonles. This is looking good :)

@ggalmazor
Copy link
Contributor

I think this is safe to be merged directly. I don't know how we could test this manually anyway. Do you agree, @lognaturel?

@yanokwa
Copy link
Member

yanokwa commented May 21, 2019

I'm not the world's greatest API designer or anything, but seems like we should be narrower here and only allow 200, 201, and 202. I don't feel strongly about it...

@terencehonles
Copy link
Contributor Author

From https://www.iana.org/assignments/http-status-codes/http-status-codes.xhtml

2xx: Success - The action was successfully received, understood, and accepted

200 | OK | [RFC7231, Section 6.3.1]
201 | Created | [RFC7231, Section 6.3.2]
202 | Accepted | [RFC7231, Section 6.3.3]
203 | Non-Authoritative Information | [RFC7231, Section 6.3.4]
204 | No Content | [RFC7231, Section 6.3.5]
205 | Reset Content | [RFC7231, Section 6.3.6]
206 | Partial Content | [RFC7233, Section 4.1]
207 | Multi-Status | [RFC4918]
208 | Already Reported | [RFC5842]
209-225 | Unassigned |  
226 | IM Used | [RFC3229]
227-299 | Unassigned

200, 201, 202, 204, 207, and 208 should be acceptable, but according to the document all of the 200 range is a "success" and there probably is not a strong reason to reject the couple that fall through and the unassigned ranges just because they don't "seem" to make sense. If the server received, understood, and accepted the request that's pretty much all the ODK server cares about.

@ggalmazor
Copy link
Contributor

I agree with @terencehonles. I think it's safe to assume any incoming 2xx response completes successfully the request.

@ggalmazor ggalmazor modified the milestones: v2.0.4, v2.0.3 Jun 3, 2019
@ggalmazor ggalmazor merged commit f79b900 into getodk:master Nov 19, 2019
@terencehonles terencehonles deleted the allow-2xx-from-json-server branch December 5, 2019 20:53
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants