search

not-an-aardvark / snoowrap

A JavaScript wrapper for the reddit API
MIT License
1.01k stars 125 forks source link

Sporadic 401s due to race condition #293

Open ELanning opened 3 years ago

ELanning commented 3 years ago 
(node:4340) UnhandledPromiseRejectionWarning: Unhandled promise rejection. This error originat
ed either by throwing inside of an async function without a catch block, or by rejecting a pro
mise which was not handled with .catch(). To terminate the node process on unhandled promise r
ejection, use the CLI flag `--unhandled-rejections=strict` (see https://nodejs.org/api/cli.htm
l#cli_unhandled_rejections_mode). (rejection id: 5)
(node:4340) UnhandledPromiseRejectionWarning: StatusCodeError: 401 - "<!doctype html><html xml
ns=\"http://www.w3.org/1999/xhtml\" lang=\"en\" xml:lang=\"en\"><head><title> ... (goes on with entire subreddit home page html)

Hypothesis: There could be a race condition where a request happens in-between a refresh interval. This could cause a 401 issue such as this.

For example, a request to get a subreddit 1ms before the access token is due to be expired, thus it expires midflight.

Solution: Handle these errors for the consumer of the API? If a 401 occurs, try to fetch a new access token once or twice before throwing a 401 error?

I am not entirely sure this is the issue as I only spent a few hours debugging, but it seems like it could be an issue in-general.

Additional info: happens sporadically with both auth methods (eg refresh_token or username/password).

Venefilyn commented 3 years ago

I don't think retrying is the way to go, but we should be able to fetch a new access token a few seconds before it expires to avoid this.

Here's the relevant code

https://github.com/not-an-aardvark/snoowrap/blob/40f6f73761898bcd60b7053bff610b6f100db0a9/src/request_handler.js#L197-L199

ELanning commented 3 years ago

That seems like it might mitigate the issue enough 👍

Of course if there's a lot of active Promises I don't know if it will get the opportunity to renew the token within that small buffer time, but I don't know if this is an issue in practice or how the internals of V8 work so this may be a non-issue.