diff --git a/lib/octaspace/client.rb b/lib/octaspace/client.rb index 4d4be18..63809ee 100644 --- a/lib/octaspace/client.rb +++ b/lib/octaspace/client.rb @@ -61,15 +61,14 @@ def shutdown # Transport diagnostics (pool stats when in keep_alive mode) # @return [Hash] def transport_stats - if @transport.respond_to?(:transport_stats) - @transport.transport_stats - elsif @transport.respond_to?(:pool_stats) - {mode: :persistent, pools: @transport.pool_stats} - elsif @config.urls.size > 1 - {mode: :standard, rotator: @transport.instance_variable_get(:@rotator)&.stats} - else - {mode: :standard, url: @config.urls.first} - end + @transport.respond_to?(:transport_stats) ? @transport.transport_stats : {mode: :custom} + end + + # Rate limit state (RateLimit-* headers) of the most recent response that + # carried them, or nil before the first such response. + # @return [OctaSpace::RateLimit, nil] + def rate_limit + @transport.respond_to?(:rate_limit) ? @transport.rate_limit : nil end private diff --git a/lib/octaspace/configuration.rb b/lib/octaspace/configuration.rb index d2029c3..2fffe8e 100644 --- a/lib/octaspace/configuration.rb +++ b/lib/octaspace/configuration.rb @@ -1,6 +1,17 @@ # frozen_string_literal: true module OctaSpace + # Test stand of the next-generation OctaSpace API. It is the default base URL + # until the production host of the new API is announced: the production hosts + # (+api.octa.computer+, +api.octa.space+) still serve the legacy API. + BASE_URL_DEV = "https://api-dev.octa.space" + + # Default API base URL + DEFAULT_BASE_URL = BASE_URL_DEV + + # Prefix of the account API keys issued in Cube (Profile → Credentials) + API_KEY_PREFIX = "octa_ak_" + # Global and per-client configuration for the OctaSpace SDK # # @example Global configuration (Rails initializer) @@ -13,15 +24,22 @@ module OctaSpace class Configuration # --- Authentication --- - # @return [String, nil] API key for Authorization header - attr_accessor :api_key + # Account API key (+octa_ak_…+), sent as +Authorization: Bearer +. + # Optional: public endpoints (network, GPU statistics, machine listings) + # work without it. A pasted value that already starts with "Bearer " is + # accepted. + # @return [String, nil] + attr_reader :api_key # --- Connection --- # @return [String] Base URL for the API (single endpoint) attr_accessor :base_url - # @return [Array, nil] Multiple API endpoints — enables URL rotation/failover + # Ordered list of API endpoints. GET requests fail over to the next URL + # on network errors; requests that change state are sent to one URL only. + # Takes precedence over {#base_url}. + # @return [Array, nil] attr_accessor :base_urls # @return [Integer] Seconds to wait for connection to open @@ -49,6 +67,9 @@ class Configuration attr_accessor :idle_timeout # --- Retry --- + # Only GET requests and the idempotent session stop are retried, on network + # errors and on 429/502/503/504. Starting or restarting a service is never + # retried: it costs money and is rate limited. # @return [Integer] Maximum number of retries on transient failures attr_accessor :max_retries @@ -59,9 +80,19 @@ class Configuration # @return [Float] Exponential backoff multiplier attr_accessor :backoff_factor + # @return [Float] Longest backoff delay in seconds (without Retry-After) + attr_accessor :max_retry_interval + + # A server-supplied Retry-After longer than this (e.g. the 5-minute + # per-session restart cooldown) is raised to the caller instead of waited for. + # @return [Integer] seconds + attr_accessor :max_retry_after + # --- Hooks --- - # @return [#call, nil] Callable invoked before each request; receives request context hash + # @return [#call, nil] Callable invoked before each request attempt; receives + # a Hash with :method, :url, :path, :params and :attempt (never the headers, + # so the API key does not leak into logs) attr_accessor :on_request # @return [#call, nil] Callable invoked after each response; receives OctaSpace::Response @@ -69,7 +100,10 @@ class Configuration # --- Logging --- - # @return [Logger, nil] Ruby Logger instance (or any object responding to #debug/#info/#warn/#error) + # Logger for one line per request (method, path, status, duration). + # Headers and bodies are never logged: they carry the API key and, + # for sessions, URLs with credentials. + # @return [Logger, nil] attr_accessor :logger # @return [Symbol] Log level (:debug, :info, :warn, :error) @@ -86,7 +120,7 @@ class Configuration attr_accessor :user_agent DEFAULTS = { - base_url: "https://api.octa.space", + base_url: DEFAULT_BASE_URL, open_timeout: 10, read_timeout: 30, write_timeout: 30, @@ -97,6 +131,8 @@ class Configuration max_retries: 2, retry_interval: 0.5, backoff_factor: 2.0, + max_retry_interval: 10.0, + max_retry_after: 60, ssl_verify: true, log_level: :info }.freeze @@ -106,6 +142,17 @@ def initialize @user_agent = "octaspace-ruby/#{OctaSpace::VERSION} Ruby/#{RUBY_VERSION}" end + # Sets the API key; surrounding whitespace and a leading "Bearer " are + # removed, and an empty value means no key. + # @param value [String, nil] + def api_key=(value) + key = value.to_s.strip.sub(/\Abearer\s+/i, "") + @api_key = key.empty? ? nil : key + end + + # @return [Boolean] whether an API key is configured + def api_key? = !api_key.nil? + # Alias: `persistent` is the Cube-internal term; `keep_alive` is the public SDK term alias_method :persistent, :keep_alive alias_method :persistent=, :keep_alive= @@ -113,12 +160,12 @@ def initialize # @return [Boolean] def keep_alive? = !!keep_alive - # Returns effective list of API URLs. + # Returns effective list of API URLs, without trailing slashes. # base_urls takes priority over base_url; always returns an Array. # @return [Array] def urls - candidates = Array(base_urls).map(&:to_s).reject(&:empty?) - candidates.empty? ? Array(base_url).map(&:to_s).reject(&:empty?) : candidates + candidates = clean_urls(base_urls) + candidates.empty? ? clean_urls(base_url) : candidates end # Deep-clone configuration for per-client overrides @@ -132,5 +179,11 @@ def dup end copy end + + private + + def clean_urls(value) + Array(value).map { |url| url.to_s.strip.sub(%r{/+\z}, "") }.reject(&:empty?) + end end end diff --git a/lib/octaspace/errors.rb b/lib/octaspace/errors.rb index ca8826e..6f4bef7 100644 --- a/lib/octaspace/errors.rb +++ b/lib/octaspace/errors.rb @@ -3,7 +3,14 @@ module OctaSpace # Base error class for all OctaSpace SDK errors class Error < StandardError - attr_reader :response, :status, :request_id + # @return [OctaSpace::Response, nil] the response that caused the error + attr_reader :response + + # @return [Integer, nil] HTTP status + attr_reader :status + + # @return [String, nil] X-Request-Id of the response + attr_reader :request_id def initialize(message = nil, response: nil) @response = response @@ -21,68 +28,78 @@ class NetworkError < Error; end class ConnectionError < NetworkError; end class TimeoutError < NetworkError; end - # API-level errors (HTTP response received, but indicates failure) - class ApiError < Error; end + # An HTTP 4xx/5xx response from the API, or a 2xx response the SDK could not + # read. The next API answers errors as +{"message": "..."}+; the message is + # used as the exception message. + class ApiError < Error + # @return [Integer, nil] seconds to wait before retrying (Retry-After header) + attr_reader :retry_after - # Domain-level rejection where transport succeeded but the provision request - # was rejected by the API payload contract. - class ProvisionRejectedError < Error - attr_reader :rejections + # @return [OctaSpace::RateLimit, nil] rate limit state of this response + attr_reader :rate_limit - def initialize(message = nil, response: nil, rejections: []) - @rejections = Array(rejections) - super(message || build_message(@rejections), response: response) + def initialize(message = nil, response: nil) + @retry_after = response&.retry_after + @rate_limit = response&.rate_limit + super end - private - - def build_message(rejections) - first_reason = - rejections.filter_map do |item| - next unless item.is_a?(Hash) - - item["reason"] || item[:reason] - end.first - - return "Provision request rejected" if first_reason.to_s.empty? - - "Provision request rejected: #{first_reason}" - end + # @return [Object, nil] parsed response body + def body = response&.data end - # 401 Unauthorized + # 400 or 422: the request was rejected as invalid; the message says why + # (for example "tail must be 1..1000" or "Invalid node number"). + class ValidationError < ApiError; end + + # 401: the API key is missing or invalid. The next API expects + # +Authorization: Bearer octa_ak_…+; legacy account tokens are refused. + # Also raised before any request when an endpoint needs a key and the + # client has none. class AuthenticationError < ApiError; end # 403 Forbidden class PermissionError < ApiError; end - # 404 Not Found + # 404: the resource does not exist. For a session this also means it has + # stopped: the API only reports running sessions. class NotFoundError < ApiError; end - # 422 Unprocessable Entity - class ValidationError < ApiError; end - - # 429 Too Many Requests — includes Retry-After header value - class RateLimitError < ApiError - attr_reader :retry_after + # 409: the operation conflicts with the current state, e.g. restarting a + # session that is still starting or restarting. + class ConflictError < ApiError; end - def initialize(message = nil, response: nil) - @retry_after = response&.retry_after - super - end - end + # 429: too many requests. Wait {#retry_after} seconds before retrying. + class RateLimitError < ApiError; end # 5xx Server Errors class ServerError < ApiError; end class BadGatewayError < ServerError; end # 502 - class ServiceUnavailableError < ServerError; end # 503 + + # 503: a backend the API depends on (render nodes, statistics, …) is + # temporarily unavailable. The request itself was fine; retry later. + class ServiceUnavailableError < ServerError; end + class GatewayTimeoutError < ServerError; end # 504 + # Legacy batch-style rejection of services.mr.create (removed with that resource) + class ProvisionRejectedError < Error + attr_reader :rejections + + def initialize(message = nil, response: nil, rejections: []) + @rejections = Array(rejections) + reason = @rejections.filter_map { |item| item["reason"] if item.is_a?(Hash) }.first + super(message || (reason ? "Provision request rejected: #{reason}" : "Provision request rejected"), response: response) + end + end + # HTTP status code → exception class mapping STATUS_ERRORS = { + 400 => ValidationError, 401 => AuthenticationError, 403 => PermissionError, 404 => NotFoundError, + 409 => ConflictError, 422 => ValidationError, 429 => RateLimitError, 502 => BadGatewayError, @@ -90,13 +107,33 @@ class GatewayTimeoutError < ServerError; end # 504 504 => GatewayTimeoutError }.freeze + # Longest error message kept from a response body (e.g. an HTML error page) + MAX_ERROR_MESSAGE_LENGTH = 512 + # Build the appropriate error for a given Response # @param response [OctaSpace::Response] + # @param api_key [String, nil] the key the request was sent with, for a hint on 401 # @return [OctaSpace::ApiError] subclass instance - def self.error_for(response) + def self.error_for(response, api_key: nil) klass = STATUS_ERRORS.fetch(response.status) do response.server_error? ? ServerError : ApiError end - klass.new(response: response) + message = error_message(response) + if response.status == 401 && api_key && !api_key.start_with?(API_KEY_PREFIX) + message += " (the next API expects a key starting with \"#{API_KEY_PREFIX}\"; " \ + "legacy account tokens are not accepted)" + end + klass.new(message, response: response) + end + + # @api private + def self.error_message(response) + data = response.data + if data.is_a?(Hash) + text = data.values_at("message", "error", "detail").find { |v| v.is_a?(String) && !v.empty? } + end + text ||= response.body.to_s.strip + text = "Request failed with status #{response.status}" if text.empty? + (text.length > MAX_ERROR_MESSAGE_LENGTH) ? "#{text[0, MAX_ERROR_MESSAGE_LENGTH]}…" : text end end diff --git a/lib/octaspace/resources/network.rb b/lib/octaspace/resources/network.rb index 2407c3b..61e6365 100644 --- a/lib/octaspace/resources/network.rb +++ b/lib/octaspace/resources/network.rb @@ -11,7 +11,7 @@ class Network < Base # GET /network # @return [OctaSpace::Response] def info - get("/network") + get("/network", auth: :none) end end end diff --git a/lib/octaspace/resources/services/machine_rental.rb b/lib/octaspace/resources/services/machine_rental.rb index 261a6cd..3b9e54f 100644 --- a/lib/octaspace/resources/services/machine_rental.rb +++ b/lib/octaspace/resources/services/machine_rental.rb @@ -19,7 +19,7 @@ class MachineRental < Base # @param params [Hash] optional filter params # @return [OctaSpace::Response] def list(**params) - get("/services/mr", params:) + get("/services/mr", params:, auth: :optional) end # Create (start) a machine rental diff --git a/lib/octaspace/resources/services/render.rb b/lib/octaspace/resources/services/render.rb index 63ab3ca..e8a39e6 100644 --- a/lib/octaspace/resources/services/render.rb +++ b/lib/octaspace/resources/services/render.rb @@ -14,7 +14,7 @@ class Render < Base # @param params [Hash] optional filter params # @return [OctaSpace::Response] def list(**params) - get("/services/render", params:) + get("/services/render", params:, auth: :optional) end # Create (start) a render job diff --git a/lib/octaspace/resources/services/session_proxy.rb b/lib/octaspace/resources/services/session_proxy.rb index e4c9670..b53f47b 100644 --- a/lib/octaspace/resources/services/session_proxy.rb +++ b/lib/octaspace/resources/services/session_proxy.rb @@ -46,7 +46,7 @@ def logs(recent: nil) # @param params [Hash] e.g. { score: 5 } # @return [OctaSpace::Response] def stop(**params) - @transport.post("/services/#{@uuid}/stop", body: params.empty? ? nil : params) + @transport.post("/services/#{@uuid}/stop", body: params.empty? ? nil : params, idempotent: true) end end end diff --git a/lib/octaspace/response.rb b/lib/octaspace/response.rb index 7127e6b..b6db90a 100644 --- a/lib/octaspace/response.rb +++ b/lib/octaspace/response.rb @@ -1,17 +1,54 @@ # frozen_string_literal: true +require "time" + module OctaSpace - # Wraps a raw Faraday response, providing a stable SDK interface + # Rate limit state reported by the API in the +RateLimit-Limit+, + # +RateLimit-Remaining+ and +RateLimit-Reset+ headers (IETF draft format, + # no +X-+ prefix). + # + # The API keeps separate windows (anonymous per IP, authenticated per + # account, and a stricter one for starting and restarting services), so the + # values describe the window of the request they came from. When a request + # is refused with 429, prefer {ApiError#retry_after}: a per-session cooldown + # can demand a longer wait than the window reset. + RateLimit = Data.define(:limit, :remaining, :reset) do + # @param headers [Hash] response headers (case-insensitive lookup) + # @return [RateLimit, nil] nil when no header is present or one is not an integer + def self.from_headers(headers) + values = %w[limit remaining reset].to_h do |name| + raw = headers["ratelimit-#{name}"].to_s.strip + return nil unless raw.empty? || raw.match?(/\A\d+\z/) + + [name.to_sym, raw.empty? ? nil : raw.to_i] + end + values.values.all?(&:nil?) ? nil : new(**values) + end + end + + # An HTTP response of the API, with the JSON body already parsed class Response - attr_reader :status, :headers, :body, :data - - # @param faraday_response [Faraday::Response] - def initialize(faraday_response) - @status = faraday_response.status - @headers = faraday_response.headers - @body = faraday_response.body - # Faraday :json middleware parses JSON body into a Hash/Array automatically - @data = faraday_response.body + # @return [Integer] HTTP status + attr_reader :status + + # @return [Hash] response headers (case-insensitive when built by the SDK transport) + attr_reader :headers + + # @return [String] raw response body + attr_reader :body + + # @return [Hash, Array, nil] parsed JSON body; nil for an empty body (e.g. 202/204) + attr_reader :data + + # @param status [Integer] + # @param headers [Hash] + # @param body [String, nil] raw body + # @param data [Hash, Array, nil] parsed JSON body + def initialize(status:, headers: {}, body: nil, data: nil) + @status = status + @headers = headers + @body = body.to_s + @data = data end # @return [Boolean] true for 2xx status codes @@ -29,8 +66,22 @@ def error? = client_error? || server_error? # @return [String, nil] X-Request-Id header value def request_id = headers["x-request-id"] - # @return [Integer, nil] Retry-After header value in seconds - def retry_after = headers["retry-after"]&.to_i + # Retry-After header as whole seconds: delta-seconds (a fractional value is + # rounded up) or an HTTP date. + # @return [Integer, nil] + def retry_after + raw = headers["retry-after"].to_s.strip + return nil if raw.empty? + return raw.to_f.ceil if raw.match?(/\A\d+(\.\d+)?\z/) + return nil unless raw.match?(/\A[A-Za-z]{3}, \d{2} [A-Za-z]{3} \d{4} \d{2}:\d{2}:\d{2} GMT\z/) + + [(Time.httpdate(raw) - Time.now).ceil, 0].max + rescue ArgumentError + nil + end + + # @return [OctaSpace::RateLimit, nil] values of the RateLimit-* headers + def rate_limit = RateLimit.from_headers(headers) def to_s "#" diff --git a/lib/octaspace/transport/base.rb b/lib/octaspace/transport/base.rb index 41acbd3..3c58b6c 100644 --- a/lib/octaspace/transport/base.rb +++ b/lib/octaspace/transport/base.rb @@ -5,47 +5,47 @@ module Transport # Abstract base class for HTTP transports # # Concrete implementations: FaradayTransport (standard) and - # PersistentTransport (keep-alive with connection pooling). + # PersistentTransport (keep-alive with connection pooling). A custom + # transport (e.g. for tests) only needs to implement {#request}. class Base - # @param path [String] - # @param params [Hash] - # @param headers [Hash] + # Perform a request and return the response, or raise an OctaSpace::Error. + # + # @param method [Symbol] :get, :post, :patch, :put or :delete + # @param path [String] API path, e.g. "/services/compute" + # @param params [Hash] query parameters (nil values are dropped) + # @param body [Hash, Array, nil] JSON body + # @param auth [Symbol] :required (raise AuthenticationError without a key), + # :optional (send the key when configured) or :none (never send it) + # @param idempotent [Boolean] allow retries for a non-GET request the API + # documents as idempotent # @return [OctaSpace::Response] - def get(path, params: {}, headers: {}) - raise NotImplementedError, "#{self.class}#get not implemented" + def request(method, path, params: {}, body: nil, auth: :required, idempotent: false) + raise NotImplementedError, "#{self.class}#request not implemented" end - # @param path [String] - # @param body [Hash, nil] - # @param headers [Hash] - # @return [OctaSpace::Response] - def post(path, body: nil, headers: {}) - raise NotImplementedError, "#{self.class}#post not implemented" + def get(path, params: {}, auth: :required) + request(:get, path, params: params, auth: auth) end - # @param path [String] - # @param body [Hash, nil] - # @param headers [Hash] - # @return [OctaSpace::Response] - def put(path, body: nil, headers: {}) - raise NotImplementedError, "#{self.class}#put not implemented" + def post(path, body: nil, auth: :required, idempotent: false) + request(:post, path, body: body, auth: auth, idempotent: idempotent) end - # @param path [String] - # @param body [Hash, nil] - # @param headers [Hash] - # @return [OctaSpace::Response] - def patch(path, body: nil, headers: {}) - raise NotImplementedError, "#{self.class}#patch not implemented" + def patch(path, body: nil, auth: :required) + request(:patch, path, body: body, auth: auth) end - # @param path [String] - # @param params [Hash] - # @param headers [Hash] - # @return [OctaSpace::Response] - def delete(path, params: {}, headers: {}) - raise NotImplementedError, "#{self.class}#delete not implemented" + def put(path, body: nil, auth: :required) + request(:put, path, body: body, auth: auth) end + + def delete(path, params: {}, auth: :required) + request(:delete, path, params: params, auth: auth) + end + + # @return [OctaSpace::RateLimit, nil] rate limit state of the most recent + # response that carried RateLimit-* headers + def rate_limit = nil end end end diff --git a/lib/octaspace/transport/faraday_transport.rb b/lib/octaspace/transport/faraday_transport.rb index 612f2c0..8325624 100644 --- a/lib/octaspace/transport/faraday_transport.rb +++ b/lib/octaspace/transport/faraday_transport.rb @@ -1,116 +1,185 @@ # frozen_string_literal: true require "faraday" -require "faraday/retry" +require "json" module OctaSpace module Transport # Standard HTTP transport built on Faraday # - # Features: - # - Automatic JSON request/response encoding - # - Configurable retry with exponential backoff + jitter - # - URL rotation / failover across multiple base_urls - # - on_request / on_response hooks - # - Typed exception hierarchy mapped from HTTP status codes + # - +Authorization: Bearer + per endpoint auth mode + # - JSON request encoding and response parsing (empty 202/204 bodies are fine) + # - Retries with exponential backoff and jitter for GET and idempotent + # requests only, honouring Retry-After up to {Configuration#max_retry_after} + # - Failover across several base URLs for GET requests on network errors; + # requests that change state are never sent to a second host + # - on_request / on_response hooks, one log line per request + # - Typed exceptions mapped from HTTP status codes # # @example # config = OctaSpace::Configuration.new - # config.api_key = "my_key" + # config.api_key = "octa_ak_..." # transport = OctaSpace::Transport::FaradayTransport.new(config) - # response = transport.get("/nodes") + # transport.get("/accounts/balance").data class FaradayTransport < Base - RETRY_STATUSES = [408, 429, 500, 502, 503, 504].freeze - RETRY_METHODS = %i[get head options put delete].freeze + # Statuses worth retrying: rate limit and transient gateway/backend failures + RETRY_STATUSES = [429, 502, 503, 504].freeze - def initialize(config) + # @param config [OctaSpace::Configuration] + # @param sleeper [#call] waits the given number of seconds between retries + def initialize(config, sleeper: ->(seconds) { sleep(seconds) }) @config = config - @rotator = (config.urls.size > 1) ? Middleware::UrlRotator.new(config.urls) : nil - # Single URL: build one connection upfront for reuse - @connection = build_connection(config.urls.first) unless @rotator - end + @urls = config.urls.freeze + raise ConfigurationError, "At least one base URL is required" if @urls.empty? + + @rotator = (@urls.size > 1) ? Middleware::UrlRotator.new(@urls) : nil + @sleeper = sleeper + @connections = {} + @mutex = Mutex.new + @rate_limit = nil + end + + # (see Base#request) + def request(method, path, params: {}, body: nil, auth: :required, idempotent: false) + method = method.to_sym + headers = build_headers(method, path, auth, body) + retryable = method == :get || idempotent + attempt = 0 + + begin + with_failover(method) do |url| + perform(url, method, path, params: params, body: body, headers: headers, attempt: attempt) + end + rescue NetworkError, ApiError => e + delay = (retryable && attempt < config.max_retries) ? retry_delay(e, attempt) : nil + raise if delay.nil? - def get(path, params: {}, headers: {}) - request(:get, path, params: params, headers: headers) + @sleeper.call(delay) if delay.positive? + attempt += 1 + retry + end end - def post(path, body: nil, headers: {}) - request(:post, path, body: body, headers: headers) - end + # (see Base#rate_limit) + def rate_limit = @mutex.synchronize { @rate_limit } - def put(path, body: nil, headers: {}) - request(:put, path, body: body, headers: headers) + # @return [Hash] transport diagnostics + def transport_stats + stats = {mode: :standard, urls: @urls} + stats[:rotator] = @rotator.stats if @rotator + stats end - def patch(path, body: nil, headers: {}) - request(:patch, path, body: body, headers: headers) - end + private - def delete(path, params: {}, headers: {}) - request(:delete, path, params: params, headers: headers) + attr_reader :config + + # GET requests try every URL in rotation order on network errors; other + # methods go to a single URL so a request that changes state (and may + # have reached the server before a timeout) is never sent twice. + def with_failover(method) + first = @rotator ? @rotator.next_url : @urls.first + urls = (method == :get) ? [first, *(@urls - [first])] : [first] + last_error = nil + + urls.each do |url| + response = yield(url) + @rotator&.mark_success(url) + return response + rescue NetworkError => e + @rotator&.mark_failed(url) + last_error = e + end + + raise last_error end - private + def perform(base_url, method, path, params:, body:, headers:, attempt:) + query = params.to_h.compact + invoke_hook(config.on_request, + {method: method, url: "#{base_url}#{path}", path: path, params: query, attempt: attempt}) - attr_reader :config + started = Process.clock_gettime(Process::CLOCK_MONOTONIC) + raw = begin + execute(connection_for(base_url), method, path.delete_prefix("/"), + query: query, payload: body.nil? ? nil : JSON.generate(body), headers: headers) + rescue Faraday::TimeoutError => e + log(method, base_url, path, "timeout", started) + raise TimeoutError, "Request timed out: #{e.message}" + rescue Faraday::ConnectionFailed, Faraday::SSLError => e + log(method, base_url, path, "connection failed", started) + raise ConnectionError, e.message + end + log(method, base_url, path, raw.status, started) - # Execute HTTP request with optional URL rotation - def request(method, path, params: {}, body: nil, headers: {}) - invoke_hook(config.on_request, {method: method, path: path, params: params}) + response = build_response(raw) + if (rate_limit = response.rate_limit) + @mutex.synchronize { @rate_limit = rate_limit } + end + invoke_hook(config.on_response, response) + raise OctaSpace.error_for(response, api_key: headers.key?("Authorization") ? config.api_key : nil) if response.error? + + response + end + + # Overridden by PersistentTransport to run the request on a pooled connection + def execute(conn, method, path, query:, payload:, headers:) + conn.run_request(method, path, payload, headers) do |req| + req.params.update(query) unless query.empty? + end + end - with_failover do |base_url| - conn = @rotator ? build_connection(base_url) : @connection + def build_response(raw) + text = raw.body.to_s + data = nil + unless text.strip.empty? begin - faraday_resp = execute(conn, method, path, params: params, body: body, headers: headers) - rescue Faraday::ConnectionFailed => e - raise ConnectionError, e.message - rescue Faraday::TimeoutError => e - raise TimeoutError, e.message + data = JSON.parse(text) + rescue JSON::ParserError + # E.g. a proxy or captive portal answering 200 with an HTML page. + if (200..299).cover?(raw.status) + response = Response.new(status: raw.status, headers: raw.headers, body: text) + raise ApiError.new("Response is not valid JSON (status #{raw.status})", response: response) + end end - response = Response.new(faraday_resp) - invoke_hook(config.on_response, response) - raise OctaSpace.error_for(response) if response.error? - response end + Response.new(status: raw.status, headers: raw.headers, body: text, data: data) end - # Execute a single Faraday request - def execute(conn, method, path, params: {}, body: nil, headers: {}) - conn.public_send(method, path) do |req| - req.headers.merge!(default_headers.merge(headers)) - req.params.merge!(params) if params.any? - req.body = body.to_json if body + def build_headers(method, path, auth, body) + headers = {"User-Agent" => config.user_agent, "Accept" => "application/json"} + headers["Content-Type"] = "application/json" unless body.nil? + + unless auth == :none + if config.api_key? + headers["Authorization"] = "Bearer #{config.api_key}" + elsif auth == :required + raise AuthenticationError, + "api_key is required for #{method.upcase} #{path}: pass an API key " \ + "(#{API_KEY_PREFIX}…) issued in Cube → Profile → Credentials" + end end + headers end - # Try each URL in rotation; mark failures and retry - def with_failover(&block) - urls = @rotator ? @config.urls.dup : [nil] - tried = [] - last_error = nil + # Seconds to wait before the next attempt, or nil when the error must be raised + def retry_delay(error, attempt) + return nil if error.is_a?(ApiError) && !RETRY_STATUSES.include?(error.status) - urls.each do |url| - next if tried.include?(url) - tried << url + retry_after = error.is_a?(ApiError) ? error.retry_after : nil + return nil if retry_after && retry_after > config.max_retry_after + return retry_after if retry_after - begin - return yield(url) - rescue ConnectionError, TimeoutError => e - last_error = e - @rotator&.mark_failed(url) - end - end + cap = [config.max_retry_interval.to_f, config.retry_interval.to_f * (config.backoff_factor.to_f**attempt)].min + rand * cap + end - raise last_error || ConnectionError.new("All API endpoints unavailable") + def connection_for(base_url) + @mutex.synchronize { @connections[base_url] ||= build_connection(base_url) } end - # Build a Faraday connection for a given base URL def build_connection(base_url) Faraday.new(url: base_url, request: request_options, ssl: ssl_options) do |f| - f.request :json - f.request :retry, retry_options - f.response :json, content_type: /\bjson/ - f.response :logger, config.logger, {headers: true, bodies: false} if config.logger configure_adapter(f) end end @@ -120,26 +189,6 @@ def configure_adapter(builder) builder.adapter :net_http end - def default_headers - headers = { - "User-Agent" => config.user_agent, - "Accept" => "application/json" - } - headers["Authorization"] = config.api_key if config.api_key && !config.api_key.empty? - headers - end - - def retry_options - { - max: config.max_retries, - interval: config.retry_interval, - interval_randomness: 0.5, - backoff_factor: config.backoff_factor, - retry_statuses: RETRY_STATUSES, - methods: RETRY_METHODS - } - end - def request_options { open_timeout: config.open_timeout, @@ -152,6 +201,14 @@ def ssl_options {verify: config.ssl_verify} end + def log(method, base_url, path, outcome, started) + return unless config.logger + + ms = ((Process.clock_gettime(Process::CLOCK_MONOTONIC) - started) * 1000).round + config.logger.public_send(config.log_level || :info, + "[OctaSpace] #{method.upcase} #{base_url}#{path} -> #{outcome} (#{ms} ms)") + end + def invoke_hook(hook, *args) hook&.call(*args) end diff --git a/lib/octaspace/transport/persistent_transport.rb b/lib/octaspace/transport/persistent_transport.rb index 42fc213..a160956 100644 --- a/lib/octaspace/transport/persistent_transport.rb +++ b/lib/octaspace/transport/persistent_transport.rb @@ -8,80 +8,54 @@ module Transport # gem "faraday-net_http_persistent" # gem "connection_pool" # - # Inherits all retry/failover/hook logic from FaradayTransport. - # Overrides adapter configuration to use net_http_persistent with pooling. + # Inherits all retry/failover/hook logic from FaradayTransport; each base + # URL gets its own pool of persistent connections. # # @example # client = OctaSpace::Client.new( - # api_key: "key", + # api_key: "octa_ak_...", # keep_alive: true, # pool_size: ENV.fetch("RAILS_MAX_THREADS", 5).to_i # ) class PersistentTransport < FaradayTransport - def initialize(config) - @pools = {} - @pools_mutex = Mutex.new - super - end - # @return [Hash] connection pool diagnostics per URL def pool_stats - @pools_mutex.synchronize do - @pools.transform_values do |pool| + @mutex.synchronize do + @connections.transform_values do |pool| {size: pool.size, available: pool.available} end end end + # @return [Hash] transport diagnostics + def transport_stats + {mode: :persistent, urls: @urls, pools: pool_stats} + end + # Shut down all connection pools gracefully def shutdown - @pools_mutex.synchronize do - @pools.each_value { |pool| pool.shutdown { |_conn| nil } } - @pools.clear + @mutex.synchronize do + @connections.each_value { |pool| pool.shutdown { |conn| conn.close if conn.respond_to?(:close) } } + @connections.clear end end private - # Override: return a ConnectionPool instead of a bare Faraday connection + # Override: a ConnectionPool of persistent Faraday connections def build_connection(base_url) - pool_for(base_url) - end - - # Execute request via pool — `pool.with` yields a Faraday connection - def execute(conn_or_pool, method, path, params: {}, body: nil, headers: {}) - if conn_or_pool.is_a?(::ConnectionPool) - conn_or_pool.with { |conn| super(conn, method, path, params: params, body: body, headers: headers) } - else - super - end - end - - def pool_for(base_url) - @pools_mutex.synchronize do - @pools[base_url] ||= build_pool(base_url) - end - end - - def build_pool(base_url) - pool_size = config.pool_size - pool_timeout = config.pool_timeout - - ::ConnectionPool.new(size: pool_size, timeout: pool_timeout) do - build_persistent_connection(base_url) + ::ConnectionPool.new(size: config.pool_size, timeout: config.pool_timeout) do + Faraday.new(url: base_url, request: request_options, ssl: ssl_options) do |f| + f.adapter :net_http_persistent, pool_size: config.pool_size do |http| + http.idle_timeout = config.idle_timeout + end + end end end - def build_persistent_connection(base_url) - Faraday.new(url: base_url, request: request_options, ssl: ssl_options) do |f| - f.request :json - f.request :retry, retry_options - f.response :json, content_type: /\bjson/ - f.response :logger, config.logger, {headers: true, bodies: false} if config.logger - f.adapter :net_http_persistent, pool_size: config.pool_size do |http| - http.idle_timeout = config.idle_timeout - end - end + # Run the request on a connection checked out of the pool + def execute(pool, method, path, query:, payload:, headers:) + pool.with { |conn| super(conn, method, path, query: query, payload: payload, headers: headers) } end end end diff --git a/octaspace.gemspec b/octaspace.gemspec index fa012a9..47b0d0c 100644 --- a/octaspace.gemspec +++ b/octaspace.gemspec @@ -27,7 +27,6 @@ Gem::Specification.new do |spec| spec.require_paths = ["lib"] spec.add_dependency "faraday", ">= 2.14.3", "< 3" - spec.add_dependency "faraday-retry", "~> 2.2" # Optional — persistent mode (users add to their own Gemfile): # gem "faraday-net_http_persistent" diff --git a/test/octaspace/configuration_test.rb b/test/octaspace/configuration_test.rb index e2f303c..1d48a2e 100644 --- a/test/octaspace/configuration_test.rb +++ b/test/octaspace/configuration_test.rb @@ -5,7 +5,7 @@ class OctaSpace::ConfigurationTest < Minitest::Test def test_defaults config = OctaSpace::Configuration.new - assert_equal "https://api.octa.space", config.base_url + assert_equal "https://api-dev.octa.space", config.base_url assert_equal 10, config.open_timeout assert_equal 30, config.read_timeout assert_equal false, config.keep_alive @@ -54,6 +54,23 @@ def test_user_agent_contains_version assert_includes config.user_agent, RUBY_VERSION end + def test_api_key_is_normalized + config = OctaSpace::Configuration.new + config.api_key = " Bearer octa_ak_x \n" + assert_equal "octa_ak_x", config.api_key + assert config.api_key? + + config.api_key = " " + assert_nil config.api_key + refute config.api_key? + end + + def test_urls_strip_trailing_slashes_and_blanks + config = OctaSpace::Configuration.new + config.base_urls = ["https://a.example/", "", " https://b.example// "] + assert_equal ["https://a.example", "https://b.example"], config.urls + end + def test_configure_block OctaSpace.configure do |c| c.api_key = "my_key" diff --git a/test/octaspace/response_test.rb b/test/octaspace/response_test.rb index 3024848..52297ca 100644 --- a/test/octaspace/response_test.rb +++ b/test/octaspace/response_test.rb @@ -4,12 +4,12 @@ class OctaSpace::ResponseTest < Minitest::Test def make_response(status:, body: {}, headers: {}) - faraday = Struct.new(:status, :headers, :body).new( - status, - {"content-type" => "application/json"}.merge(headers), - body + OctaSpace::Response.new( + status: status, + headers: Faraday::Utils::Headers.new({"content-type" => "application/json"}.merge(headers)), + body: JSON.generate(body), + data: body ) - OctaSpace::Response.new(faraday) end # --- Status predicates --- @@ -70,11 +70,34 @@ def test_retry_after_nil_when_missing # --- Data --- - def test_data_is_body + def test_data_is_parsed_body body = {"id" => 1, "name" => "test"} r = make_response(status: 200, body: body) assert_equal body, r.data - assert_equal body, r.body + assert_equal JSON.generate(body), r.body + end + + def test_retry_after_fraction_rounds_up + assert_equal 2, make_response(status: 429, headers: {"Retry-After" => "1.2"}).retry_after + end + + def test_retry_after_http_date + date = (Time.now + 30).httpdate + assert_in_delta 30, make_response(status: 429, headers: {"Retry-After" => date}).retry_after, 2 + end + + def test_retry_after_garbage_is_nil + assert_nil make_response(status: 429, headers: {"Retry-After" => "soon"}).retry_after + end + + def test_rate_limit_from_headers + r = make_response(status: 200, headers: {"RateLimit-Limit" => "300", "RateLimit-Remaining" => "12"}) + assert_equal OctaSpace::RateLimit.new(limit: 300, remaining: 12, reset: nil), r.rate_limit + end + + def test_rate_limit_nil_without_headers_or_with_garbage + assert_nil make_response(status: 200).rate_limit + assert_nil make_response(status: 200, headers: {"RateLimit-Limit" => "many"}).rate_limit end def test_data_array diff --git a/test/octaspace/transport/faraday_transport_test.rb b/test/octaspace/transport/faraday_transport_test.rb index 86fed61..6bcfdf5 100644 --- a/test/octaspace/transport/faraday_transport_test.rb +++ b/test/octaspace/transport/faraday_transport_test.rb @@ -1,218 +1,351 @@ # frozen_string_literal: true require "test_helper" +require "logger" +require "stringio" class OctaSpace::FaradayTransportTest < Minitest::Test BASE = StubHelpers::BASE_URL + KEY = "octa_ak_testkey" + JSON_HEADERS = {"Content-Type" => "application/json"}.freeze def setup super + @sleeps = [] @config = OctaSpace::Configuration.new - @config.api_key = "test_key" + @config.api_key = KEY @config.max_retries = 0 - @transport = OctaSpace::Transport::FaradayTransport.new(@config) end - # --- Basic HTTP verbs --- + def transport(config = @config) + OctaSpace::Transport::FaradayTransport.new(config, sleeper: ->(s) { @sleeps << s }) + end + + # --- Requests --- + + def test_get_parses_json_body + stub_request(:get, "#{BASE}/sessions").to_return(status: 200, body: '[{"uuid":"a"}]', headers: JSON_HEADERS) + response = transport.get("/sessions") - def test_get_returns_response - stub_request(:get, "#{BASE}/nodes") - .to_return(status: 200, body: "[]", headers: {"Content-Type" => "application/json"}) - response = @transport.get("/nodes") assert_instance_of OctaSpace::Response, response - assert response.success? + assert_equal [{"uuid" => "a"}], response.data + end + + def test_post_sends_json_body + stub_request(:post, "#{BASE}/services/compute") + .with(body: '{"node_number":"M95S-WZG8-7"}', headers: {"Content-Type" => "application/json"}) + .to_return(status: 201, body: '{"uuid":"u"}', headers: JSON_HEADERS) + + assert_equal "u", transport.post("/services/compute", body: {node_number: "M95S-WZG8-7"}).data["uuid"] end - def test_post_returns_response - stub_request(:post, "#{BASE}/idle-jobs") - .to_return(status: 201, body: '{"id":1}', headers: {"Content-Type" => "application/json"}) - response = @transport.post("/idle-jobs", body: {command: "sleep 1"}) - assert response.success? + def test_empty_body_on_204_and_202 + stub_request(:patch, "#{BASE}/hosting/nodes/X/prices").to_return(status: 204, body: "") + stub_request(:post, "#{BASE}/services/u/stop").to_return(status: 202, body: "") + + assert_nil transport.patch("/hosting/nodes/X/prices", body: {base: 100}).data + assert_nil transport.post("/services/u/stop").data end - def test_put_returns_response - stub_request(:put, "#{BASE}/nodes/1/prices") - .to_return(status: 200, body: "{}", headers: {"Content-Type" => "application/json"}) - response = @transport.put("/nodes/1/prices", body: {gpu_hour: 0.5}) - assert response.success? + def test_get_drops_nil_query_params + stub_request(:get, "#{BASE}/services/u/logs").with(query: {"tail" => "10"}).to_return(status: 200, body: "{}") + assert transport.get("/services/u/logs", params: {tail: 10, recent: nil}).success? end - def test_delete_returns_response - stub_request(:delete, "#{BASE}/idle-jobs/1") - .to_return(status: 200, body: "{}", headers: {"Content-Type" => "application/json"}) - response = @transport.delete("/idle-jobs/1") - assert response.success? + def test_base_url_path_prefix_is_kept + @config.base_url = "https://proxy.example/octa/" + stub_request(:get, "https://proxy.example/octa/network").to_return(status: 200, body: "{}") + + assert transport.get("/network", auth: :none).success? + end + + def test_invalid_json_on_success_raises_api_error + stub_request(:get, "#{BASE}/network").to_return(status: 200, body: "portal") + + error = assert_raises(OctaSpace::ApiError) { transport.get("/network", auth: :none) } + assert_includes error.message, "not valid JSON" + assert_equal 200, error.status + end + + # --- Authorization --- + + def test_sends_bearer_authorization + stub_request(:get, "#{BASE}/accounts/balance") + .with(headers: {"Authorization" => "Bearer #{KEY}"}) + .to_return(status: 200, body: "{}") + + assert transport.get("/accounts/balance").success? end - # --- Authorization header --- + def test_pasted_bearer_prefix_is_not_doubled + @config.api_key = " Bearer #{KEY} " + stub_request(:get, "#{BASE}/accounts/balance") + .with(headers: {"Authorization" => "Bearer #{KEY}"}) + .to_return(status: 200, body: "{}") - def test_sends_authorization_header - stub_request(:get, "#{BASE}/nodes") - .with(headers: {"Authorization" => "test_key"}) - .to_return(status: 200, body: "[]", headers: {"Content-Type" => "application/json"}) - @transport.get("/nodes") - # WebMock raises if header doesn't match — reaching here means it did - assert true + assert transport.get("/accounts/balance").success? + end + + def test_required_auth_without_key_raises_before_request + @config.api_key = nil + + error = assert_raises(OctaSpace::AuthenticationError) { transport.get("/sessions") } + assert_includes error.message, "api_key is required for GET /sessions" + assert_not_requested :get, "#{BASE}/sessions" + end + + def test_optional_auth_sends_key_when_present_and_skips_without + stub_request(:get, "#{BASE}/services/compute").to_return(status: 200, body: "[]") + + transport.get("/services/compute", auth: :optional) + assert_requested :get, "#{BASE}/services/compute", headers: {"Authorization" => "Bearer #{KEY}"} + + @config.api_key = nil + transport.get("/services/compute", auth: :optional) + assert_requested(:get, "#{BASE}/services/compute", times: 1) { |req| !req.headers.key?("Authorization") } + end + + def test_auth_none_never_sends_key + stub_request(:get, "#{BASE}/network").to_return(status: 200, body: "{}") + transport.get("/network", auth: :none) + + assert_requested(:get, "#{BASE}/network") { |req| !req.headers.key?("Authorization") } + end + + def test_401_with_legacy_key_adds_hint + @config.api_key = "a" * 64 + stub_request(:get, "#{BASE}/accounts/balance").to_return(status: 401, body: '{"message":"Invalid bearer token"}') + + error = assert_raises(OctaSpace::AuthenticationError) { transport.get("/accounts/balance") } + assert_match(/\AInvalid bearer token \(the next API expects a key starting with "octa_ak_"/, error.message) + end + + def test_401_with_new_key_has_no_hint + stub_request(:get, "#{BASE}/accounts/balance").to_return(status: 401, body: '{"message":"Invalid bearer token"}') + + error = assert_raises(OctaSpace::AuthenticationError) { transport.get("/accounts/balance") } + assert_equal "Invalid bearer token", error.message end # --- Error mapping --- - def test_raises_authentication_error_on_401 - stub_request(:get, "#{BASE}/accounts") - .to_return(status: 401, body: '{"error":"Unauthorized"}', - headers: {"Content-Type" => "application/json"}) - assert_raises(OctaSpace::AuthenticationError) { @transport.get("/accounts") } + { + 400 => OctaSpace::ValidationError, + 401 => OctaSpace::AuthenticationError, + 403 => OctaSpace::PermissionError, + 404 => OctaSpace::NotFoundError, + 409 => OctaSpace::ConflictError, + 422 => OctaSpace::ValidationError, + 429 => OctaSpace::RateLimitError, + 500 => OctaSpace::ServerError, + 502 => OctaSpace::BadGatewayError, + 503 => OctaSpace::ServiceUnavailableError, + 504 => OctaSpace::GatewayTimeoutError, + 418 => OctaSpace::ApiError + }.each do |status, klass| + define_method(:"test_status_#{status}_raises_#{klass.name.split("::").last}") do + stub_request(:get, "#{BASE}/sessions").to_return(status: status, body: '{"message":"boom"}') + + error = assert_raises(klass) { transport.get("/sessions") } + assert_equal status, error.status + assert_equal "boom", error.message + assert_equal({"message" => "boom"}, error.body) + end + end + + def test_error_message_falls_back_to_text_and_is_truncated + stub_request(:get, "#{BASE}/sessions").to_return(status: 502, body: "x" * 600) + + error = assert_raises(OctaSpace::BadGatewayError) { transport.get("/sessions") } + assert_equal 513, error.message.length + end + + def test_error_carries_retry_after_rate_limit_and_request_id + stub_request(:post, "#{BASE}/services/compute/u/restart").to_return( + status: 429, + body: '{"message":"Session was restarted too recently, retry in 288 seconds"}', + headers: {"Retry-After" => "288", "RateLimit-Limit" => "10", "RateLimit-Remaining" => "6", + "RateLimit-Reset" => "41", "X-Request-Id" => "req-1"} + ) + + error = assert_raises(OctaSpace::RateLimitError) { transport.post("/services/compute/u/restart") } + assert_equal 288, error.retry_after + assert_equal OctaSpace::RateLimit.new(limit: 10, remaining: 6, reset: 41), error.rate_limit + assert_equal "req-1", error.request_id end - def test_raises_permission_error_on_403 - stub_request(:get, "#{BASE}/accounts") - .to_return(status: 403, body: '{"error":"Forbidden"}', - headers: {"Content-Type" => "application/json"}) - assert_raises(OctaSpace::PermissionError) { @transport.get("/accounts") } + # --- Rate limit state --- + + def test_rate_limit_from_last_response + t = transport + assert_nil t.rate_limit + + stub_request(:get, "#{BASE}/sessions").to_return(status: 200, body: "[]", + headers: {"RateLimit-Limit" => "300", "RateLimit-Remaining" => "299", "RateLimit-Reset" => "60"}) + t.get("/sessions") + + assert_equal 299, t.rate_limit.remaining end - def test_raises_not_found_error_on_404 - stub_request(:get, "#{BASE}/nodes/99") - .to_return(status: 404, body: '{"error":"Not found"}', - headers: {"Content-Type" => "application/json"}) - assert_raises(OctaSpace::NotFoundError) { @transport.get("/nodes/99") } + # --- Retries --- + + def test_get_retries_on_503_with_backoff + @config.max_retries = 2 + stub_request(:get, "#{BASE}/network") + .to_return({status: 503, body: "{}"}, {status: 503, body: "{}"}, {status: 200, body: "{}"}) + + assert transport.get("/network", auth: :none).success? + assert_equal 2, @sleeps.size + assert(@sleeps.all? { |s| s.between?(0, @config.max_retry_interval) }) end - def test_raises_validation_error_on_422 - stub_request(:post, "#{BASE}/idle-jobs") - .to_return(status: 422, body: '{"error":"Invalid params"}', - headers: {"Content-Type" => "application/json"}) - assert_raises(OctaSpace::ValidationError) { @transport.post("/idle-jobs", body: {}) } + def test_get_retry_honours_retry_after + @config.max_retries = 1 + stub_request(:get, "#{BASE}/network") + .to_return({status: 429, body: "{}", headers: {"Retry-After" => "7"}}, {status: 200, body: "{}"}) + + transport.get("/network", auth: :none) + assert_equal [7], @sleeps end - def test_raises_rate_limit_error_on_429_with_retry_after - stub_request(:get, "#{BASE}/nodes") - .to_return(status: 429, body: "{}", - headers: {"Content-Type" => "application/json", "Retry-After" => "30"}) - error = assert_raises(OctaSpace::RateLimitError) { @transport.get("/nodes") } - assert_equal 30, error.retry_after + def test_long_retry_after_is_raised_not_waited + @config.max_retries = 3 + stub_request(:get, "#{BASE}/network").to_return(status: 429, body: "{}", headers: {"Retry-After" => "300"}) + + error = assert_raises(OctaSpace::RateLimitError) { transport.get("/network", auth: :none) } + assert_equal 300, error.retry_after + assert_empty @sleeps + assert_requested :get, "#{BASE}/network", times: 1 end - def test_retries_on_429_then_raises_after_exhaustion - # Enable retry with Retry-After: 0 so sleep is instant + def test_retries_stop_after_max_retries @config.max_retries = 1 - transport = OctaSpace::Transport::FaradayTransport.new(@config) - stub_request(:get, "#{BASE}/nodes") - .to_return(status: 429, body: "{}", - headers: {"Content-Type" => "application/json", "Retry-After" => "0"}) - assert_raises(OctaSpace::RateLimitError) { transport.get("/nodes") } - assert_requested :get, "#{BASE}/nodes", times: 2 + stub_request(:get, "#{BASE}/network").to_return(status: 429, body: "{}", headers: {"Retry-After" => "0"}) + + assert_raises(OctaSpace::RateLimitError) { transport.get("/network", auth: :none) } + assert_requested :get, "#{BASE}/network", times: 2 end - def test_raises_server_error_on_500 - stub_request(:get, "#{BASE}/nodes") - .to_return(status: 500, body: '{"error":"Internal Server Error"}', - headers: {"Content-Type" => "application/json"}) - assert_raises(OctaSpace::ServerError) { @transport.get("/nodes") } + def test_get_retries_network_errors + @config.max_retries = 1 + stub_request(:get, "#{BASE}/network") + .to_raise(Faraday::ConnectionFailed.new("refused")).then + .to_return(status: 200, body: "{}") + + assert transport.get("/network", auth: :none).success? end - def test_raises_bad_gateway_error_on_502 - stub_request(:get, "#{BASE}/nodes") - .to_return(status: 502, body: "{}", headers: {"Content-Type" => "application/json"}) - assert_raises(OctaSpace::BadGatewayError) { @transport.get("/nodes") } + def test_client_errors_are_not_retried + @config.max_retries = 2 + stub_request(:get, "#{BASE}/sessions").to_return(status: 404, body: "{}") + + assert_raises(OctaSpace::NotFoundError) { transport.get("/sessions") } + assert_requested :get, "#{BASE}/sessions", times: 1 end - def test_raises_service_unavailable_on_503 - stub_request(:get, "#{BASE}/nodes") - .to_return(status: 503, body: "{}", headers: {"Content-Type" => "application/json"}) - assert_raises(OctaSpace::ServiceUnavailableError) { @transport.get("/nodes") } + def test_post_is_never_retried + @config.max_retries = 2 + stub_request(:post, "#{BASE}/services/compute").to_return(status: 503, body: "{}") + stub_request(:post, "#{BASE}/services/compute/u/restart").to_raise(Faraday::TimeoutError.new("read timeout")) + + assert_raises(OctaSpace::ServiceUnavailableError) { transport.post("/services/compute", body: {}) } + assert_raises(OctaSpace::TimeoutError) { transport.post("/services/compute/u/restart") } + assert_requested :post, "#{BASE}/services/compute", times: 1 + assert_requested :post, "#{BASE}/services/compute/u/restart", times: 1 end - def test_raises_gateway_timeout_on_504 - stub_request(:get, "#{BASE}/nodes") - .to_return(status: 504, body: "{}", headers: {"Content-Type" => "application/json"}) - assert_raises(OctaSpace::GatewayTimeoutError) { @transport.get("/nodes") } + def test_idempotent_post_is_retried + @config.max_retries = 1 + stub_request(:post, "#{BASE}/services/u/stop") + .to_return({status: 502, body: ""}, {status: 202, body: '{"uuid":"u"}'}) + + assert_equal 202, transport.post("/services/u/stop", idempotent: true).status end # --- Network errors --- - def test_raises_connection_error_on_connection_failed - stub_request(:get, "#{BASE}/nodes").to_raise(Faraday::ConnectionFailed.new("Connection refused")) - assert_raises(OctaSpace::ConnectionError) { @transport.get("/nodes") } + def test_connection_failed_maps_to_connection_error + stub_request(:get, "#{BASE}/network").to_raise(Faraday::ConnectionFailed.new("Connection refused")) + assert_raises(OctaSpace::ConnectionError) { transport.get("/network", auth: :none) } end - def test_raises_timeout_error_on_timeout - stub_request(:get, "#{BASE}/nodes").to_raise(Faraday::TimeoutError.new("timeout")) - assert_raises(OctaSpace::TimeoutError) { @transport.get("/nodes") } + def test_timeout_maps_to_timeout_error + stub_request(:get, "#{BASE}/network").to_raise(Faraday::TimeoutError.new("read timeout")) + assert_raises(OctaSpace::TimeoutError) { transport.get("/network", auth: :none) } end - # --- Hooks --- + # --- Failover --- + + def test_get_fails_over_to_next_url + @config.base_urls = ["https://bad.example", "https://good.example"] + stub_request(:get, "https://bad.example/network").to_raise(Faraday::ConnectionFailed.new("refused")) + stub_request(:get, "https://good.example/network").to_return(status: 200, body: "{}") - def test_on_request_hook_is_called - called_with = nil - @config.on_request = ->(ctx) { called_with = ctx } - stub_request(:get, "#{BASE}/nodes") - .to_return(status: 200, body: "[]", headers: {"Content-Type" => "application/json"}) - @transport.get("/nodes") - assert_equal :get, called_with[:method] - assert_equal "/nodes", called_with[:path] + assert transport.get("/network", auth: :none).success? end - def test_on_response_hook_is_called - called_with = nil - @config.on_response = ->(resp) { called_with = resp } - stub_request(:get, "#{BASE}/nodes") - .to_return(status: 200, body: "[]", headers: {"Content-Type" => "application/json"}) - @transport.get("/nodes") - assert_instance_of OctaSpace::Response, called_with - assert_equal 200, called_with.status + def test_get_raises_when_all_urls_fail + @config.base_urls = ["https://bad1.example", "https://bad2.example"] + stub_request(:get, "https://bad1.example/network").to_raise(Faraday::ConnectionFailed.new("refused")) + stub_request(:get, "https://bad2.example/network").to_raise(Faraday::ConnectionFailed.new("refused")) + + assert_raises(OctaSpace::ConnectionError) { transport.get("/network", auth: :none) } end - # --- URL rotation / failover --- + def test_post_is_not_sent_to_a_second_url + @config.base_urls = ["https://bad.example", "https://good.example"] + stub_request(:post, "https://bad.example/services/compute").to_raise(Faraday::TimeoutError.new("read timeout")) + good = stub_request(:post, "https://good.example/services/compute").to_return(status: 201, body: "{}") - def test_url_rotation_with_multiple_base_urls - @config.base_urls = ["https://api.octa.space", "https://api2.octa.space"] - transport = OctaSpace::Transport::FaradayTransport.new(@config) + assert_raises(OctaSpace::TimeoutError) { transport.post("/services/compute", body: {}) } + assert_not_requested good + end - stub_request(:get, "https://api.octa.space/nodes") - .to_return(status: 200, body: "[]", headers: {"Content-Type" => "application/json"}) - stub_request(:get, "https://api2.octa.space/nodes") - .to_return(status: 200, body: "[]", headers: {"Content-Type" => "application/json"}) + def test_http_error_does_not_fail_over + @config.base_urls = ["https://a.example", "https://b.example"] + stub_request(:get, "https://a.example/sessions").to_return(status: 404, body: "{}") + other = stub_request(:get, "https://b.example/sessions").to_return(status: 200, body: "[]") - # Both calls should succeed (rotator picks one) - resp1 = transport.get("/nodes") - resp2 = transport.get("/nodes") - assert resp1.success? - assert resp2.success? + assert_raises(OctaSpace::NotFoundError) { transport.get("/sessions") } + assert_not_requested other end - def test_failover_to_second_url_on_connection_error - @config.base_urls = ["https://bad.octa.space", "https://api.octa.space"] - transport = OctaSpace::Transport::FaradayTransport.new(@config) + # --- Hooks and logging --- - stub_request(:get, "https://bad.octa.space/nodes") - .to_raise(Faraday::ConnectionFailed.new("refused")) - stub_request(:get, "https://api.octa.space/nodes") - .to_return(status: 200, body: "[]", headers: {"Content-Type" => "application/json"}) + def test_on_request_hook_gets_context_without_headers + seen = [] + @config.on_request = ->(ctx) { seen << ctx } + stub_request(:get, "#{BASE}/network").to_return(status: 200, body: "{}") + transport.get("/network", auth: :none) - response = transport.get("/nodes") - assert response.success? + assert_equal :get, seen.first[:method] + assert_equal "/network", seen.first[:path] + assert_equal "#{BASE}/network", seen.first[:url] + assert_equal 0, seen.first[:attempt] + refute seen.first.key?(:headers) end - def test_raises_connection_error_when_all_urls_fail - @config.base_urls = ["https://bad1.octa.space", "https://bad2.octa.space"] - transport = OctaSpace::Transport::FaradayTransport.new(@config) + def test_on_response_hook_gets_response + seen = nil + @config.on_response = ->(resp) { seen = resp } + stub_request(:get, "#{BASE}/network").to_return(status: 200, body: "{}") + transport.get("/network", auth: :none) - stub_request(:get, "https://bad1.octa.space/nodes") - .to_raise(Faraday::ConnectionFailed.new("refused")) - stub_request(:get, "https://bad2.octa.space/nodes") - .to_raise(Faraday::ConnectionFailed.new("refused")) - - assert_raises(OctaSpace::ConnectionError) { transport.get("/nodes") } + assert_equal 200, seen.status end - # --- Query params --- + def test_logger_writes_one_line_without_the_key + io = StringIO.new + @config.logger = Logger.new(io) + stub_request(:get, "#{BASE}/accounts/balance").to_return(status: 200, body: "{}") + transport.get("/accounts/balance") + + assert_match(%r{\[OctaSpace\] GET #{Regexp.escape(BASE)}/accounts/balance -> 200 \(\d+ ms\)}o, io.string) + refute_includes io.string, KEY + end - def test_get_passes_query_params - stub_request(:get, "#{BASE}/nodes") - .with(query: {"state" => "online", "limit" => "10"}) - .to_return(status: 200, body: "[]", headers: {"Content-Type" => "application/json"}) - response = @transport.get("/nodes", params: {state: "online", limit: 10}) - assert response.success? + def test_requires_a_base_url + @config.base_url = " " + assert_raises(OctaSpace::ConfigurationError) { transport } end end diff --git a/test/octaspace/transport/persistent_transport_test.rb b/test/octaspace/transport/persistent_transport_test.rb index 85b5f9d..3d80ca9 100644 --- a/test/octaspace/transport/persistent_transport_test.rb +++ b/test/octaspace/transport/persistent_transport_test.rb @@ -38,6 +38,10 @@ def test_post_returns_response assert response.success? end + def test_transport_stats_reports_persistent_mode + assert_equal :persistent, @transport.transport_stats[:mode] + end + def test_pool_stats_returns_hash stats = @transport.pool_stats assert_kind_of Hash, stats @@ -76,9 +80,8 @@ def test_raises_authentication_error_on_401 def test_sends_authorization_header stub_request(:get, "#{BASE}/nodes") - .with(headers: {"Authorization" => "test_key"}) + .with(headers: {"Authorization" => "Bearer test_key", "Connection" => "keep-alive"}) .to_return(status: 200, body: "[]", headers: {"Content-Type" => "application/json"}) - @transport.get("/nodes") - assert true + assert @transport.get("/nodes").success? end end diff --git a/test/support/stub_helpers.rb b/test/support/stub_helpers.rb index ff8911d..791e379 100644 --- a/test/support/stub_helpers.rb +++ b/test/support/stub_helpers.rb @@ -3,7 +3,7 @@ require "webmock/minitest" module StubHelpers - BASE_URL = "https://api.octa.space" + BASE_URL = OctaSpace::DEFAULT_BASE_URL # Stub a successful GET request # @param path [String] API path (e.g. "/nodes")