Skip to content

Commit

Permalink
fix(cts): add tests for HTML error (#4097)
Browse files Browse the repository at this point in the history
  • Loading branch information
millotp authored Nov 19, 2024
1 parent 14989b8 commit f97e44c
Show file tree
Hide file tree
Showing 14 changed files with 101 additions and 13 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -84,6 +84,9 @@ private Response handleResponse(StatefulHost host, @Nonnull Response response) t

try {
String message = response.body() != null ? response.body().string() : response.message();
if (response.header("Content-Type", "application/json").contains("text/html")) {
message = response.message();
}
throw isRetryable(response)
? new AlgoliaRequestException(message, response.code())
: new AlgoliaApiException(message, response.code());
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -82,6 +82,29 @@ export function deserializeSuccess<TObject>(response: Response): TObject {
}
}

const httpMessages: Record<number, string> = {
400: 'Bad Request',
401: 'Unauthorized',
402: 'Payment Required',
403: 'Forbidden',
404: 'Not Found',
405: 'Method Not Allowed',
406: 'Not Acceptable',
407: 'Proxy Authentication Required',
408: 'Request Timeout',
409: 'Conflict',
410: 'Gone',
411: 'Length Required',
412: 'Precondition Required',
413: 'Request Entry Too Large',
414: 'Request-URI Too Long',
415: 'Unsupported Media Type',
416: 'Requested Range Not Satisfiable',
417: 'Expectation Failed',
418: 'I\'m a teapot',
429: 'Too Many Requests',
}

export function deserializeFailure({ content, status }: Response, stackFrame: StackFrame[]): Error {
try {
const parsed = JSON.parse(content);
Expand All @@ -92,5 +115,5 @@ export function deserializeFailure({ content, status }: Response, stackFrame: St
} catch {
// ..
}
return new ApiError(content, status, stackFrame);
return new ApiError(status in httpMessages ? httpMessages[status] : content, status, stackFrame);
}
3 changes: 2 additions & 1 deletion clients/algoliasearch-client-php/lib/Http/CurlHttpClient.php
Original file line number Diff line number Diff line change
Expand Up @@ -116,11 +116,12 @@ public function sendRequest(
$statusCode = (int) curl_getinfo($curlHandle, CURLINFO_HTTP_CODE);
$responseBody = curl_multi_getcontent($curlHandle);
$error = curl_error($curlHandle);
$contentType = curl_getinfo($curlHandle, CURLINFO_CONTENT_TYPE);

$this->releaseMHandle($curlHandle);
curl_close($curlHandle);

return new Response($statusCode, [], $responseBody, '1.1', $error);
return new Response($statusCode, ['Content-Type' => $contentType], $responseBody, '1.1', $error);
}

private function getMHandle($curlHandle)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -247,6 +247,11 @@ private function handleResponse(
throw new RetriableException('Retriable failure on '.$request->getUri()->getHost().': '.$reason, $statusCode);
}

// handle HTML error responses
if (false !== strpos($response->getHeaderLine('Content-Type'), 'text/html')) {
throw new AlgoliaException($statusCode.': '.$response->getReasonPhrase(), $statusCode);
}

$responseArray = Helpers::json_decode($body, true);

if (404 === $statusCode) {
Expand Down
6 changes: 3 additions & 3 deletions clients/algoliasearch-client-ruby/lib/algolia/error.rb
Original file line number Diff line number Diff line change
Expand Up @@ -27,12 +27,12 @@ def initialize(message, errors = [])
# which is also included in the response attribute.
#
class AlgoliaHttpError < AlgoliaError
attr_accessor :code, :message
attr_accessor :code, :http_message

def initialize(code, message)
self.code = code
self.message = message
super("#{self.code()}: #{self.message()}")
self.http_message = message
super("#{code}: #{message}")
end
end
end
Original file line number Diff line number Diff line change
Expand Up @@ -39,14 +39,14 @@ def send_request(host, method, path, body, query_params, headers, timeout, conne
@logger.info("Request succeeded. Response status: #{response.status}, body: #{response.body}")
end

return Http::Response.new(status: response.status, body: response.body, headers: response.headers)
return Http::Response.new(status: response.status, reason_phrase: response.reason_phrase, body: response.body, headers: response.headers)
end

if ENV["ALGOLIA_DEBUG"]
@logger.info("Request failed. Response status: #{response.status}, error: #{response.body}")
end

Http::Response.new(status: response.status, error: response.body, headers: response.headers)
Http::Response.new(status: response.status, reason_phrase: response.reason_phrase, error: response.body, headers: response.headers)
rescue Faraday::TimeoutError => e
@logger.info("Request timed out. Error: #{e.message}") if ENV["ALGOLIA_DEBUG"]
Http::Response.new(error: e.message, has_timed_out: true)
Expand Down
Original file line number Diff line number Diff line change
@@ -1,20 +1,22 @@
module Algolia
module Http
class Response
attr_reader :status, :body, :error, :headers, :has_timed_out, :network_failure
attr_reader :status, :reason_phrase, :body, :error, :headers, :has_timed_out, :network_failure

# used for the echo requester
attr_reader :method, :path, :query_params, :host, :timeout, :connect_timeout

#
# @option status [String] Response status
# @option reason_phrase [String] Response reason phrase
# @option body [String] Response body
# @option error [String] Response error or caught error
# @option headers [String] Response headers
# @option has_timed_out [String] If the request has timed out
#
def initialize(opts = {})
@status = opts[:status]
@reason_phrase = opts[:reason_phrase]
@body = opts[:body]
@error = opts[:error] || ""
@headers = opts[:headers] || ""
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -69,6 +69,11 @@ def request(call_type, method, path, body, opts = {})
network_failure: response.network_failure
)
if outcome == FAILURE
# handle HTML error
if response.headers["content-type"]&.include?("text/html")
raise Algolia::AlgoliaHttpError.new(response.status, response.reason_phrase)
end

decoded_error = JSON.parse(response.error, :symbolize_names => true)
raise Algolia::AlgoliaHttpError.new(response.status, decoded_error[:message])
end
Expand Down
5 changes: 5 additions & 0 deletions scripts/cts/testServer/timeout.ts
Original file line number Diff line number Diff line change
Expand Up @@ -97,6 +97,11 @@ function addRoutes(app: express.Express): void {

// no response, just hang
});

app.get('/1/html-error', (req, res) => {
res.setHeader('Content-Type', 'text/html');
res.status(429).send('<html><body>429 Too Many Requests</body></html>');
});
}

export function timeoutServer(): Promise<Server> {
Expand Down
2 changes: 1 addition & 1 deletion templates/ruby/tests/client/tests.mustache
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@
{{#dynamicTemplate}}{{/dynamicTemplate}}
assert(false, 'An error should have been raised')
rescue => e
assert_equal({{#lambda.codeSnakeCase}}'{{{expectedError}}}'{{/lambda.codeSnakeCase}}.sub('%localhost%', ENV.fetch('CI', nil) == 'true' ? 'localhost' : 'host.docker.internal'), e.message)
assert_equal({{#lambda.codeSnakeCase}}%q({{{expectedError}}}){{/lambda.codeSnakeCase}}.sub('%localhost%', ENV.fetch('CI', nil) == 'true' ? 'localhost' : 'host.docker.internal'), e.message)
end
{{/isError}}
{{^isError}}
Expand Down
42 changes: 42 additions & 0 deletions tests/CTS/client/ingestion/api.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,42 @@
[
{
"testName": "can handle HTML error",
"autoCreateClient": false,
"steps": [
{
"type": "createClient",
"parameters": {
"appId": "test-app-id",
"apiKey": "test-api-key",
"region": "us",
"customHosts": [
{
"port": 6676
}
]
}
},
{
"type": "method",
"method": "customGet",
"parameters": {
"path": "1/html-error"
},
"expected": {
"error": {
"csharp": "<html><body>429 too many requests</body></html>",
"go": "API error [429] Too Many Requests",
"java": "Status Code: 429 - Too Many Requests",
"javascript": "Too Many Requests",
"kotlin": "Client request(GET http://%localhost%:6676/1/html-error) invalid: 429 Too Many Requests. Text: \\\"<html><body>429 Too Many Requests</body></html>\\\"",
"php": "429: Too Many Requests",
"python": "Too Many Requests",
"ruby": "429: Too Many Requests",
"scala": "<html><body>429 Too Many Requests</body></html>",
"swift": "HTTP error: Status code: 429 Message: No message"
}
}
}
]
}
]
2 changes: 1 addition & 1 deletion tests/CTS/client/search/indexExists.json
Original file line number Diff line number Diff line change
Expand Up @@ -88,7 +88,7 @@
"kotlin": "Client request(GET http://%localhost%:6681/1/indexes/indexExistsERROR/settings) invalid: 403 Forbidden. Text: \\\"{\\\"message\\\":\\\"Invalid API key\\\"}\\\"",
"php": "Invalid API key",
"python": "Invalid API key",
"ruby": "Invalid API key",
"ruby": "403: Invalid API key",
"swift": "HTTP error: Status code: 403 Message: Invalid API key"
}
}
Expand Down
3 changes: 1 addition & 2 deletions tests/CTS/client/search/saveObjects.json
Original file line number Diff line number Diff line change
Expand Up @@ -87,8 +87,7 @@
"kotlin": "Client request(POST http://%localhost%:6680/1/indexes/cts_e2e_saveObjects_kotlin/batch) invalid: 403 Forbidden. Text: \\\"{\\\"message\\\":\\\"Invalid Application-ID or API key\\\",\\\"status\\\":403}\\\"",
"php": "Invalid Application-ID or API key",
"python": "Invalid Application-ID or API key",
"ruby": "Invalid Application-ID or API key",
"scala": "Invalid Application-ID or API key",
"ruby": "403: Invalid Application-ID or API key",
"swift": "HTTP error: Status code: 403 Message: Invalid Application-ID or API key"
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,10 @@ package object algoliasearch {

def assertError(message: String)(call: => Unit)(implicit ec: ExecutionContextExecutor): Unit = {
val error = intercept[Exception](call)
assert(error.getMessage == message)
assert(
error.getMessage == message,
s"Error message does not match, expected: $message, got: ${error.getMessage}"
)
}

@targetName("assertErrorFuture")
Expand Down

0 comments on commit f97e44c

Please sign in to comment.