aerokube / selenoid

Selenium Hub successor running browsers within containers. Scalable, immutable, self hosted Selenium-Grid on any platform with single binary.
https://aerokube.com/selenoid/latest/
Apache License 2.0
2.6k stars 324 forks source link

Incorrect status code in case of "Too many requests" response #1203

Open eGavr opened 2 years ago

eGavr commented 2 years ago

Selenoid version: 1.10.7 Broken since selenoid version: 1.10.7 Run selenoid with options -disable-queue -limit 5

Current behaviour:

If queue is full selenoid returns an unknown error with status code 500

Expected behaviour:

If queue is full selenoid returns an error with status code 429 (too many requests)

JFYI. In out team we've hacked the problem this way.

vania-pooh commented 2 years ago

@eGavr no such status code in W3C standard. https://www.w3.org/TR/webdriver/#errors

eGavr commented 2 years ago

@eGavr no such status code in W3C standard. https://www.w3.org/TR/webdriver/#errors

Yes, you are right.

But how can we determine that the error reason is "Too many requests"? This value is in error message, but I think relying for this value in error message is not a good idea.

vania-pooh commented 2 years ago

@eGavr should rely on error field which is constant.

eGavr commented 2 years ago

The full error is

{"name":"HTTPError","hostname":"127.0.0.1","method":"POST","path":"/wd/hub/session","protocol":"http:","url":"http://127.0.0.1:4444/wd/hub/session","gotOptions":{"path":"/wd/hub/session","protocol":"http:","hostname":"127.0.0.1","hash":"","search":"","pathname":"/wd/hub/session","href":"http://127.0.0.1:4444/wd/hub/session","port":4444,"retry":{"methods":{},"statusCodes":{},"errorCodes":{},"maxRetryAfter":60001},"headers":{"user-agent":"@yandex-int/si.ci.requests@3.0.12","accept":"application/json","accept-encoding":"gzip, deflate","content-type":"application/json","x-request-id":"27a71ca75354ec64b0f3706bce94200c","content-length":107},"hooks":{"beforeError":[],"init":[],"beforeRequest":[],"beforeRedirect":[],"beforeRetry":[],"afterResponse":[]},"decompress":true,"throwHttpErrors":true,"followRedirect":true,"stream":false,"form":false,"json":true,"cache":false,"useElectronNet":false,"nonRetriableStatusCodes":[],"retryCount":4,"applyMaxRetryTotalTimeBeforeRetry":true,"truncateResponseBodyLogAfter":500,"gotTimeout":{"request":60001},"method":"POST","baseUrl":"http://127.0.0.1:4444/","body":"{\"desiredCapabilities\":{\"screenResolution\":\"1600x1200x24\",\"browserName\":\"firefox\",\"browserVersion\":\"95.0\"}}"},"statusCode":500,"statusMessage":"Internal Server Error","headers":{"content-type":"application/json; charset=utf-8","date":"Mon, 28 Mar 2022 13:55:57 GMT","content-length":"62","connection":"close","x-request-id":"27a71ca75354ec64b0f3706bce94200c"},"body":{"value":{"error":"unknown error","message":"Queue Is Full"}}}

"unknown error" is not concrete error which cannot be associated with the original reason about "Queue Is Full" (I mean that in my program code I need to distinguish unknown errors and error because of full queue)

vania-pooh commented 2 years ago

@eGavr probably we can introduce a new error code that is not included to the standard.

eGavr commented 2 years ago

Until version 1.10.7 selenoid has an error that is not included to the standard, and this error was with status code 429 :)

aandryashin commented 2 years ago

That error code needs only for ggr, and can be disabled by command line option. Selenoid was developed when w3c standard did not exist.

вт, 29 мар. 2022 г., 10:46 Evgeniy Gavryushin @.***>:

Until version 1.10.7 selenoid has an error that is not included to the standard, and this error was with status code 429 :)

— Reply to this email directly, view it on GitHub https://github.com/aerokube/selenoid/issues/1203#issuecomment-1081539764, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAKY23NEZBTCKDHJ2V6Y763VCKYOPANCNFSM5ROFTUDA . You are receiving this because you are subscribed to this thread.Message ID: @.***>

eGavr commented 2 years ago

@eGavr probably we can introduce a new error code that is not included to the standard.

@vania-pooh @aandryashin

Can we do this?

To tell the truth, it seems that since status code 429 disappeared backward compatibility has been broken in 1.x