From e0f0c502188614c86361721c59e4e45ac71934c2 Mon Sep 17 00:00:00 2001 From: Alexander Puzynia Date: Wed, 16 Sep 2026 00:29:45 -0700 Subject: [PATCH] Fail fast when Alpha Vantage refuses a request for the account Alpha Vantage answers a premium-only endpoint (TIME_SERIES_DAILY_ADJUSTED on a free key) with HTTP 200 and an "Information" message instead of a time series. The parser read that as invalid data and the retry policy ran through the whole back-off budget before failing. A premium-endpoint refusal or an invalid API key now throws the new FinanceNetAccessDeniedException from every Alpha Vantage call. Like FinanceNetNoDataException it derives from FinanceNetException, is not retried and is not wrapped. The invalid-key check was previously only in GetForexRecordsAsync and was retried. The rate-limit message stays retryable. --- CLAUDE.md | 4 +- src/Constants.cs | 1 + .../FinanceNetAccessDeniedException.cs | 41 ++++++ src/Services/AlphaVantageService.cs | 34 +++-- src/Utilities/PollyPolicyFactory.cs | 7 +- .../AlphaVantageRejectedRequestTests.cs | 118 ++++++++++++++++++ 6 files changed, 192 insertions(+), 13 deletions(-) create mode 100644 src/Exceptions/FinanceNetAccessDeniedException.cs create mode 100644 tests/Services/AlphaVantageRejectedRequestTests.cs diff --git a/CLAUDE.md b/CLAUDE.md index 7550d43..bb0a912 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -34,14 +34,14 @@ Tests use NUnit + Moq and target net10.0. Three categories gate what runs: `Unit Entry point is `ServiceCollectionExtensions.AddFinanceNet()` (src/Extensions), which wires everything: -- Four scoped services in `src/Services/` behind public interfaces in `src/Interfaces/`: `YahooFinanceService`, `AlphaVantageService`, `XetraService`, `DataHubService`. Services are `internal`; only interfaces, models, enums, config (`FinanceNetConfiguration`), and the exception are public API. +- Four scoped services in `src/Services/` behind public interfaces in `src/Interfaces/`: `YahooFinanceService`, `AlphaVantageService`, `XetraService`, `DataHubService`. Services are `internal`; only interfaces, models, enums, config (`FinanceNetConfiguration`), and the exceptions are public API. - One named `HttpClient` per provider (names in `src/Constants.cs`, which also holds all provider URLs). Each client gets a randomized User-Agent (`Helper.CreateRandomUserAgent`). - A shared Polly retry policy (`PollyPolicyFactory`) registered in a `PolicyRegistry` under `Constants.DefaultHttpRetryPolicy`; services resolve it from the registry and wrap their HTTP calls with it. Retry count and back-off base (`HttpRetryCount`, `HttpRetrySleepTime`) come from `FinanceNetConfiguration`; `HttpTimeout` is the HTTP client timeout only and must not be reused as the back-off base. The back-off grows exponentially from the base, is capped at `PollyPolicyFactory.MaxRetryDelaySecs` (30s) per attempt, and carries jitter of up to one full delay. - Internal DTOs (`src/Models/*/Dtos/`) are converted to the public models (`src/Models/*/`) by hand-written extension-method mappers in `src/Mappings/` (`YahooQuoteMapper.ToQuote`, `XetraInstrumentMapper.ToInstrument`). The `*Mapping.cs` classes in the same folder are CsvHelper `ClassMap`s, not object-to-object mappers. Yahoo requires session state: `YahooSessionState` and `YahooSessionManager` (singletons in `src/Utilities/`) fetch consent cookies and an API "crumb", guarded by a semaphore, valid for 6 hours. The Yahoo HttpClient shares the session's `CookieContainer`. HTML parsing uses AngleSharp with XPath (`YahooHtmlParser`); Alpha Vantage JSON parsing is in `AlphaVantageParser`; CSV parsing uses CsvHelper. -All failures surface as `FinanceNetException`. A provider that answered correctly but with no rows throws `FinanceNetNoDataException` (which derives from it) and is deliberately **not** wrapped, so `InnerException` is null there; the retry policy skips it. Everything else - transport errors, an unexpected page, malformed content - stays retryable and keeps its inner exception. +All failures surface as `FinanceNetException`. A provider that answered correctly but with no rows throws `FinanceNetNoDataException` (which derives from it) and is deliberately **not** wrapped, so `InnerException` is null there; the retry policy skips it. The same applies to `FinanceNetAccessDeniedException`, thrown when the provider refuses the request for the account (Alpha Vantage premium-only endpoint, invalid API key). Everything else - transport errors, an unexpected page, malformed content - stays retryable and keeps its inner exception. ## Conventions diff --git a/src/Constants.cs b/src/Constants.cs index d6d9ae2..5739236 100644 --- a/src/Constants.cs +++ b/src/Constants.cs @@ -15,6 +15,7 @@ internal static class Constants // Messages public const string ApiResponseLimitExceeded = "higher API call volume"; public const string ApiResponseApiKeyInvalid = "apikey is invalid"; + public const string ApiResponsePremiumEndpoint = "premium endpoint"; #region Yahoo Finance diff --git a/src/Exceptions/FinanceNetAccessDeniedException.cs b/src/Exceptions/FinanceNetAccessDeniedException.cs new file mode 100644 index 0000000..423aa7d --- /dev/null +++ b/src/Exceptions/FinanceNetAccessDeniedException.cs @@ -0,0 +1,41 @@ +using System; + +namespace Finance.Net.Exceptions; + +/// +/// Represents a provider refusing the request for the configured account - an endpoint +/// that needs a premium plan, an invalid or missing API key. +/// +/// +/// This is a permanent answer, not a transient transport failure: the retry policy does +/// not retry it. It derives from , so existing handlers +/// that catch keep working unchanged. +/// +public class FinanceNetAccessDeniedException : FinanceNetException +{ + /// + /// Initializes a new instance of the class. + /// + public FinanceNetAccessDeniedException() + { + } + + /// + /// Initializes a new instance of the class with a specified error message. + /// + /// The message that describes the error. + public FinanceNetAccessDeniedException(string message) + : base(message) + { + } + + /// + /// Initializes a new instance of the class with a specified error message and a reference to the inner exception that is the cause of this exception. + /// + /// The message that describes the error. + /// The exception that is the cause of the current exception. + public FinanceNetAccessDeniedException(string message, Exception inner) + : base(message, inner) + { + } +} diff --git a/src/Services/AlphaVantageService.cs b/src/Services/AlphaVantageService.cs index 9e77a28..57eee20 100644 --- a/src/Services/AlphaVantageService.cs +++ b/src/Services/AlphaVantageService.cs @@ -80,6 +80,7 @@ public AlphaVantageService( httpResponse.EnsureSuccessStatusCode(); var jsonResponse = await httpResponse.Content.ReadAsStringAsync().ConfigureAwait(false); + ThrowIfRejected(jsonResponse); if (jsonResponse.Contains(Constants.ApiResponseLimitExceeded)) { throw new FinanceNetException($"{Constants.ApiResponseLimitExceeded} for {symbol}"); @@ -97,7 +98,7 @@ public AlphaVantageService( { throw; } - catch (Exception ex) when (ex is not FinanceNetNoDataException) + catch (Exception ex) when (ex is not (FinanceNetNoDataException or FinanceNetAccessDeniedException)) { throw new FinanceNetException($"No overview found for {symbol}", ex); } @@ -126,6 +127,7 @@ public async Task> GetRecordsAsync(string symbol, DateTime? return await _retryPolicy.ExecuteAsync(async ct => { var jsonResponse = await Helper.FetchJsonDocumentAsync(httpClient, _logger, url, ct).ConfigureAwait(false); + ThrowIfRejected(jsonResponse); if (jsonResponse.Contains(Constants.ApiResponseLimitExceeded)) { throw new FinanceNetException($"{Constants.ApiResponseLimitExceeded} for {symbol}"); @@ -138,7 +140,7 @@ public async Task> GetRecordsAsync(string symbol, DateTime? { throw; } - catch (Exception ex) when (ex is not FinanceNetNoDataException) + catch (Exception ex) when (ex is not (FinanceNetNoDataException or FinanceNetAccessDeniedException)) { throw new FinanceNetException($"No Record found for {symbol}", ex); } @@ -197,6 +199,7 @@ private async Task> GetIntradayRecordsByMonthAsync(string s response.EnsureSuccessStatusCode(); var jsonResponse = await response.Content.ReadAsStringAsync().ConfigureAwait(false); + ThrowIfRejected(jsonResponse); if (jsonResponse.Contains(Constants.ApiResponseLimitExceeded)) { throw new FinanceNetException($"{Constants.ApiResponseLimitExceeded} for {symbol}"); @@ -209,7 +212,7 @@ private async Task> GetIntradayRecordsByMonthAsync(string s { throw; } - catch (Exception ex) when (ex is not FinanceNetNoDataException) + catch (Exception ex) when (ex is not (FinanceNetNoDataException or FinanceNetAccessDeniedException)) { throw new FinanceNetException($"No intraday record found for {symbol}", ex); } @@ -240,14 +243,11 @@ public async Task> GetForexRecordsAsync(string currency return await _retryPolicy.ExecuteAsync(async ct => { var jsonResponse = await Helper.FetchJsonDocumentAsync(httpClient, _logger, url, ct).ConfigureAwait(false); + ThrowIfRejected(jsonResponse); if (jsonResponse.Contains(Constants.ApiResponseLimitExceeded)) { throw new FinanceNetException($"{Constants.ApiResponseLimitExceeded} for {currency1} /{currency2}"); } - if (jsonResponse.Contains(Constants.ApiResponseApiKeyInvalid)) - { - throw new FinanceNetException($"{Constants.ApiResponseApiKeyInvalid}"); - } var result = AlphaVantageParser.ParseForexRecords(currency1, currency2, startDate, endDate, jsonResponse, _logger); return result.IsNullOrEmpty() ? throw new FinanceNetNoDataException($"Alpha Vantage returned no forex records for {currency1}/{currency2}") : result; }, token).ConfigureAwait(false); @@ -256,9 +256,27 @@ public async Task> GetForexRecordsAsync(string currency { throw; } - catch (Exception ex) when (ex is not FinanceNetNoDataException) + catch (Exception ex) when (ex is not (FinanceNetNoDataException or FinanceNetAccessDeniedException)) { throw new FinanceNetException($"No forex record found for {currency1}, {currency2}", ex); } } + + /// + /// Alpha Vantage answers a refused request with HTTP 200 and a JSON message instead of data. + /// A premium-only endpoint or a bad API key will not change on retry, so fail immediately. + /// + private static void ThrowIfRejected(string jsonResponse) + { + if (jsonResponse.Contains(Constants.ApiResponsePremiumEndpoint)) + { + throw new FinanceNetAccessDeniedException( + $"Alpha Vantage rejected the request: this is a {Constants.ApiResponsePremiumEndpoint} and needs a premium plan"); + } + if (jsonResponse.Contains(Constants.ApiResponseApiKeyInvalid)) + { + throw new FinanceNetAccessDeniedException( + $"Alpha Vantage rejected the request: {Constants.ApiResponseApiKeyInvalid} or missing"); + } + } } diff --git a/src/Utilities/PollyPolicyFactory.cs b/src/Utilities/PollyPolicyFactory.cs index 8011d1c..a3a10b1 100644 --- a/src/Utilities/PollyPolicyFactory.cs +++ b/src/Utilities/PollyPolicyFactory.cs @@ -15,9 +15,10 @@ internal static class PollyPolicyFactory public static AsyncRetryPolicy GetRetryPolicy(int retryCount, int baseWaitTimeSecs, ILogger logger) { return Policy - // A provider that answered with no data has given a permanent answer - retrying - // it burns the whole back-off budget and cannot change the outcome. - .Handle(ex => ex is not FinanceNetNoDataException) + // A provider that answered with no data, or refused the request for this account, + // has given a permanent answer - retrying it burns the whole back-off budget and + // cannot change the outcome. + .Handle(ex => ex is not (FinanceNetNoDataException or FinanceNetAccessDeniedException)) .WaitAndRetryAsync( retryCount, retryAttempt => GetRetryDelay(retryAttempt, baseWaitTimeSecs), diff --git a/tests/Services/AlphaVantageRejectedRequestTests.cs b/tests/Services/AlphaVantageRejectedRequestTests.cs new file mode 100644 index 0000000..4a72fba --- /dev/null +++ b/tests/Services/AlphaVantageRejectedRequestTests.cs @@ -0,0 +1,118 @@ +using System; +using System.Net; +using System.Net.Http; +using System.Text; +using System.Threading; +using System.Threading.Tasks; +using Finance.Net.Exceptions; +using Finance.Net.Services; +using Finance.Net.Utilities; +using Microsoft.Extensions.Logging; +using Microsoft.Extensions.Options; +using Moq; +using Moq.Protected; +using NUnit.Framework; +using Polly; +using Polly.Registry; + +namespace Finance.Net.Tests.Services; + +/// +/// Alpha Vantage refusing a request for the account (premium-only endpoint, invalid API key) +/// is a permanent answer: retrying cannot change it. It must surface as +/// and cost exactly one request. +/// +[TestFixture] +[Category("Unit")] +public class AlphaVantageRejectedRequestTests +{ + private const int RetryCount = 3; + private const string PremiumResponse = + "{\"Information\": \"Thank you for using Alpha Vantage! This is a premium endpoint. You may subscribe to any of the premium plans at https://www.alphavantage.co/premium/ to instantly unlock all premium endpoints\"}"; + private const string InvalidApiKeyResponse = + "{\"Error Message\": \"the parameter apikey is invalid or missing. Please claim your free API key on (https://www.alphavantage.co/support/#api-key). It should take less than 20 seconds.\"}"; + + private int _requestCount; + private AlphaVantageService _service; + + private void SetUpResponse(string body) + { + _requestCount = 0; + var mockHandler = new Mock(); + mockHandler + .Protected() + .Setup>("SendAsync", ItExpr.IsAny(), ItExpr.IsAny()) + .Returns(() => + { + _requestCount++; + return Task.FromResult(new HttpResponseMessage(HttpStatusCode.OK) + { + Content = new StringContent(body, Encoding.UTF8, "application/json"), + }); + }); + + var mockHttpClientFactory = new Mock(); + mockHttpClientFactory.Setup(e => e.CreateClient(It.IsAny())).Returns(() => new HttpClient(mockHandler.Object)); + + IAsyncPolicy policy = PollyPolicyFactory.GetRetryPolicy(RetryCount, 0, null); + var mockPolicyRegistry = new Mock>(); + mockPolicyRegistry.Setup(registry => registry.TryGet(Constants.DefaultHttpRetryPolicy, out policy)).Returns(true); + + var mockOptions = new Mock>(); + mockOptions.Setup(x => x.Value).Returns(new FinanceNetConfiguration()); + + _service = new AlphaVantageService( + Mock.Of>(), + mockHttpClientFactory.Object, + mockOptions.Object, + mockPolicyRegistry.Object); + } + + private static readonly DateTime StartDate = new(2026, 7, 1, 0, 0, 0, DateTimeKind.Utc); + + private static readonly object[] Calls = + [ + new object[] { "GetRecordsAsync", (Func)(s => s.GetRecordsAsync("IBM", StartDate)) }, + new object[] { "GetOverviewAsync", (Func)(s => s.GetOverviewAsync("IBM")) }, + new object[] { "GetIntradayRecordsAsync", (Func)(s => s.GetIntradayRecordsAsync("IBM", StartDate, StartDate.AddDays(1))) }, + new object[] { "GetForexRecordsAsync", (Func)(s => s.GetForexRecordsAsync("EUR", "USD", StartDate)) }, + ]; + + [TestCaseSource(nameof(Calls))] + public void PremiumEndpoint_FailsFastWithoutRetrying(string name, Func call) + { + SetUpResponse(PremiumResponse); + + var exception = Assert.CatchAsync(async () => await call(_service), name); + Assert.That(exception.Message, Does.Contain("premium endpoint"), name); + Assert.That(exception.InnerException, Is.Null, name); + Assert.That(_requestCount, Is.EqualTo(1), $"{name}: a premium-endpoint refusal was retried"); + } + + [TestCaseSource(nameof(Calls))] + public void InvalidApiKey_FailsFastWithoutRetrying(string name, Func call) + { + SetUpResponse(InvalidApiKeyResponse); + + var exception = Assert.CatchAsync(async () => await call(_service), name); + Assert.That(exception.Message, Does.Contain("apikey is invalid"), name); + Assert.That(_requestCount, Is.EqualTo(1), $"{name}: an invalid-API-key refusal was retried"); + } + + [Test] + public void ApiLimitExceeded_IsStillRetried() + { + // The per-minute limit clears on its own, so it stays a retryable failure. + SetUpResponse("{\"Information\": \"Please consider spreading out your free API requests more sparingly (1 request per second). You may subscribe to any of the premium plans at https://www.alphavantage.co/premium/ to lift the free key rate limit, or visit higher API call volume.\"}"); + + var exception = Assert.CatchAsync(async () => await _service.GetRecordsAsync("IBM", StartDate)); + Assert.That(exception, Is.Not.InstanceOf()); + Assert.That(_requestCount, Is.EqualTo(RetryCount + 1)); + } + + [Test] + public void FinanceNetAccessDeniedException_IsAFinanceNetException() + { + Assert.That(new FinanceNetAccessDeniedException("denied"), Is.InstanceOf()); + } +}