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

Fix status code 401 symbol usage in doc capture polling #4292

Closed
wants to merge 1 commit into from

Conversation

aduth
Copy link
Member

@aduth aduth commented Oct 8, 2020

Why: So that the server returns the expected status code (401) and doesn't error on an unrecognized status code

There doesn't appear to be much test coverage for this behavior, but I will plan to include some. I'd discovered this in the course of implementing tests for LG-3423 (dedicated controller for doc capture polling).

Reference: https://guides.rubyonrails.org/layouts_and_rendering.html#the-status-option

Because the front-end polling behavior checks only for 200 status code, it likely does not have a user-facing impact:

request.onload = function () {
// This endpoint renders a 202 for a pending request
if (request.status === 200) {
docCaptureComplete();
}
};

However, the use of an unrecognized status code symbol could produce errors in the server response.

**Why**: So that the server returns the expected status code (401) and doesn't error on an unrecognized status code
@aduth
Copy link
Member Author

aduth commented Oct 8, 2020

I created this as a separate pull request since I'd worried this might have been a regression in recent revisions to document capture, though it appears this behavior has existed since the initial introduction of polling in March (#3646). If it's not proven to have been too problematic, this could wait for the refactoring work of LG-3423?

@zachmargolis
Copy link
Contributor

There doesn't appear to be much test coverage for this behavior, but I will plan to include some

Good catch, thanks for tests --- very surprised we didn't catch this sooner 😬

@aduth
Copy link
Member Author

aduth commented Oct 9, 2020

This fix is absorbed into #4293.

We could still choose to cherry-pick this change if we decide to try to patch RC 120.

@aduth aduth closed this Oct 9, 2020
@aduth aduth deleted the aduth-doc-auth-poll-unauthorized branch January 26, 2021 15:50
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants