Skip to content

chore: improve error handling for device code exchange failure #11708

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

Merged

Conversation

Emyrk
Copy link
Member

@Emyrk Emyrk commented Jan 19, 2024

What this does

Error handling on device code exchanges now:

  • Looks at header for github rate limit info to specify how long to wait for.
  • Checks for application/x-www-form-urlencoded header which means our request Accept header was not checked
  • If all else fails, return status code and mime type info for debugging.

This was all information we wanted when debugging the failures from github, so it's all valuable info that can be hit in prod.

Test extra

Implemented a test to show how to use the new idp device flow. Honestly in practice, the current method is easier, so using this is a bit overkill. I mainly implemented it so I can test the UI, which is much harder to mock up.

This does actually run our device auth flow in a unit test though, which has value, so I am going to keep it.

Notes

I noticed we have the UI poll for the device auth to finish, I wonder if we should just do this with a websocket on the FE, and have the BE control the rate of request.

Copy link
Member Author

Emyrk commented Jan 19, 2024

Warning

This pull request is not mergeable via GitHub because a downstack PR is open. Once all requirements are satisfied, merge this PR as a stack on Graphite.
Learn more

This stack of pull requests is managed by Graphite. Learn more about stacking.

Join @Emyrk and the rest of your teammates on Graphite Graphite

@Emyrk Emyrk changed the title chore: work on unit test for device auth chore: implement a unit test for device auth with fake idp Jan 19, 2024
@Emyrk Emyrk marked this pull request as ready for review January 19, 2024 18:11
@Emyrk Emyrk changed the title chore: implement a unit test for device auth with fake idp chore: improve error handling for device code exchange failure Jan 19, 2024
@Emyrk Emyrk force-pushed the stevenmasley/fake_device_flow branch from b72cea9 to da21464 Compare January 19, 2024 18:33
@Emyrk Emyrk force-pushed the stevenmasley/device_flow_response_type branch from 6229dc1 to 7bf5f67 Compare January 19, 2024 18:33
@Emyrk Emyrk merged commit f88117e into stevenmasley/fake_device_flow Jan 19, 2024
@Emyrk Emyrk deleted the stevenmasley/device_flow_response_type branch January 19, 2024 20:12
@github-actions github-actions bot locked and limited conversation to collaborators Jan 19, 2024
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.

2 participants