Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Failed validation: #/definitions/QuoteResponse #32

Closed
pudgereyem opened this issue Feb 10, 2021 · 5 comments · Fixed by #33
Closed

Failed validation: #/definitions/QuoteResponse #32

pudgereyem opened this issue Feb 10, 2021 · 5 comments · Fixed by #33
Labels

Comments

@pudgereyem
Copy link
Contributor

Issue

Yahoo's quote endpoint returns different data depending on what time a day it is, e.g quote.marketState is a string that described the state of the market. In the current version it's allowed to be either "CLOSED" or "PREPRE". However, at the time of this writing - when the market is closed - quote.marketState was set to "POST" and then later it was set to "POSTPOST".

How to replicate

Run the integration tests for the quote module when the markets (e.g NASDAQ) are closed. Please also make sure that you disable caching by turning of devel mode for the "passing validation" tests (see code here).

Please note that the response from Yahoo is different depending on the time of day, so make sure that you test this when the markets are closed, e.g 20:00 New York Time.

npm run test --t quote.spec.ts

 FAIL  src/modules/quote.spec.ts
  quote
    ✓ allows blank options (7 ms)
    ✓ returns an array for an array (4 ms)
    ✓ returns single for a string (1 ms)
    ✓ throws on unexpected result (5 ms)
    passes validation
      ✕ AAPL (419 ms)
      ✓ OCDO.L (70 ms)
      ✕ BABA (79 ms)
      ✕ QQQ (73 ms)
      ✕ 0P000071W8.TO (77 ms)
Click to see the error for AAPL
    The following result did not validate with schema: #/definitions/QuoteResponse

      at Object.validate [as default] (src/lib/validateAndCoerceTypes.ts:140:15)

  console.dir
    [
      {
        keyword: 'enum',
        dataPath: '/0/marketState',
        schemaPath: '#/definitions/QuoteEquity/properties/marketState/enum',
        params: { allowedValues: [Array] },
        message: 'should be equal to one of the allowed values'
      },
      {
        keyword: 'enum',
        dataPath: '/0/marketState',
        schemaPath: '#/definitions/QuoteEtf/properties/marketState/enum',
        params: { allowedValues: [Array] },
        message: 'should be equal to one of the allowed values'
      },
      {
        keyword: 'const',
        dataPath: '/0/quoteType',
        schemaPath: '#/definitions/QuoteEtf/properties/quoteType/const',
        params: { allowedValue: 'ETF' },
        message: 'should be equal to constant'
      },
      {
        keyword: 'enum',
        dataPath: '/0/marketState',
        schemaPath: '#/definitions/QuoteMutualfund/properties/marketState/enum',
        params: { allowedValues: [Array] },
        message: 'should be equal to one of the allowed values'
      },
      {
        keyword: 'const',
        dataPath: '/0/quoteType',
        schemaPath: '#/definitions/QuoteMutualfund/properties/quoteType/const',
        params: { allowedValue: 'MUTUALFUND' },
        message: 'should be equal to constant'
      },
      {
        keyword: 'anyOf',
        dataPath: '/0',
        schemaPath: '#/anyOf',
        params: {},
        message: 'should match some schema in anyOf'
      }
    ] { depth: 4, colors: true }

      at src/lib/validateAndCoerceTypes.ts:112:26

  console.dir
    [
      {
        language: 'en-US',
        region: 'US',
        quoteType: 'EQUITY',
        quoteSourceName: 'Nasdaq Real Time Price',
        triggerable: true,
        currency: 'USD',
        exchange: 'NMS',
        shortName: 'Apple Inc.',
        longName: 'Apple Inc.',
        postMarketPrice: 135.98,
        postMarketChange: -0.02999878,
        regularMarketChange: -0.90000916,
        regularMarketChangePercent: -0.65737283,
        regularMarketTime: 1612904402,
        regularMarketPrice: 136.01,
        regularMarketDayHigh: 137.877,
        regularMarketDayRange: { low: 135.85, high: 137.877 },
        regularMarketDayLow: 135.85,
        regularMarketVolume: 73445499,
        regularMarketPreviousClose: 136.91,
        bid: 135.84,
        ask: 135.85,
        bidSize: 13,
        askSize: 29,
        fullExchangeName: 'NasdaqGS',
        financialCurrency: 'USD',
        regularMarketOpen: 136.62,
        averageDailyVolume3Month: 106252653,
        averageDailyVolume10Day: 85075233,
        fiftyTwoWeekLowChange: 82.8575,
        fiftyTwoWeekLowChangePercent: 1.5588636,
        fiftyTwoWeekRange: { low: 53.1525, high: 145.09 },
        fiftyTwoWeekHighChange: -9.080002,
        fiftyTwoWeekHighChangePercent: -0.06258186,
        fiftyTwoWeekLow: 53.1525,
        fiftyTwoWeekHigh: 145.09,
        dividendDate: 2021-02-11T00:00:00.000Z,
        earningsTimestamp: 2021-01-27T16:30:00.000Z,
        earningsTimestampStart: 2021-04-28T10:59:00.000Z,
        earningsTimestampEnd: 2021-05-03T12:00:00.000Z,
        trailingAnnualDividendRate: 0.807,
        trailingPE: 36.88907,
        trailingAnnualDividendYield: 0.005894383,
        epsTrailingTwelveMonths: 3.687,
        epsForward: 4.66,
        epsCurrentYear: 4.45,
        priceEpsCurrentYear: 30.564045,
        sharesOutstanding: 16788100096,
        bookValue: 3.936,
        fiftyDayAverage: 133.41939,
        fiftyDayAverageChange: 2.5906067,
        fiftyDayAverageChangePercent: 0.019417018,
        twoHundredDayAverage: 120.43009,
        twoHundredDayAverageChange: 15.579903,
        twoHundredDayAverageChangePercent: 0.12936886,
        marketCap: 2283349475328,
        forwardPE: 29.186695,
        priceToBook: 34.555386,
        sourceInterval: 15,
        exchangeDataDelayedBy: 0,
        tradeable: false,
        firstTradeDateMilliseconds: 1980-12-12T14:30:00.000Z,
        priceHint: 2,
        postMarketChangePercent: -0.022056306,
        postMarketTime: 2021-02-10T00:59:45.000Z,
        messageBoardId: 'finmb_24937',
        exchangeTimezoneName: 'America/New_York',
        exchangeTimezoneShortName: 'EST',
        gmtOffSetMilliseconds: -18000000,
        market: 'us_market',
        esgPopulated: false,
        marketState: 'POSTPOST',
        displayName: 'Apple',
        symbol: 'AAPL'
      }
    ] { depth: 4, colors: true }

      at src/lib/validateAndCoerceTypes.ts:112:26

Proposed Solution

Since the response from Yahoo's /quote-endpoint can return a response where marketState is set to "POST" or "POSTPOST" we should add these ass possible values for the marketState property for the QuoteBase interface.

export interface QuoteBase {
  ...
  // marketState: "CLOSED" | "PREPRE"; // <-- current 
  marketState: "CLOSED" | "PREPRE" | "POST" | "POSTPOST"; // <-- suggested
  ...
}

Comments / Discussion

Caching the responses can hide issues like this where the response from Yahoo differs depending on the time of day

Since Yahoo's response is different depending on the time of the day, it's important that while in development we turn of devel mode every now and then. I was thinking that we could add parameter support to the test script so that you could run the tests with devel mode turned of, maybe something like this:

yarn test --live
@pudgereyem
Copy link
Contributor Author

pudgereyem commented Feb 10, 2021

@gadicc I'll create a PR according to the proposed solution above, but I'd love to hear your thoughts on the yarn test --live option. I'll also try to run the tests with caching turned at different times to see if there are more issues like this.

@pudgereyem
Copy link
Contributor Author

@gadicc please see PR that fixes the issue with marketState here; #33.

Let's create a separate issue for how we best deal with running the test suite with devel mode turned off. Sadly you can't pass custom arguments to jest, so maybe the best option would be to add environment variables through .env which we could take into account in yahooFinanceFetch.js possibly?

@gadicc
Copy link
Owner

gadicc commented Feb 10, 2021

Thanks, @pudgereyem. I think we might have a bunch of these type errors which we'll have some short term pain with but will reap the benefits for years after to get it right now (will discuss also in the other issue). So thanks for fixing this, I'll say also that for simple things like this you can just submit a PR without a whole write up (unless you enjoy it :) just trying to save you some time).

Will address the caching issue elsewhere.

@pudgereyem
Copy link
Contributor Author

Hey @gadicc, I agree! And yeah, I might start creating PR's without issues going forward. However, I sort of like creating issues because it gives me time to "focus on the problem" rather than "implementing a solution". It's also a good practice for documentation in its own way I believe.

Super!

@gadicc
Copy link
Owner

gadicc commented Feb 10, 2021

🎉 This issue has been resolved in version 1.7.0 🎉

The release is available on:

Your semantic-release bot 📦🚀

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants