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

fix_382 #383

Merged
merged 10 commits into from
Jun 14, 2023
Merged

Conversation

ethanbsmith
Copy link
Contributor

@ethanbsmith ethanbsmith commented Apr 19, 2023

adds a lightweight session for use on yahoo quote requests
session is needed for both the cookies and the "crumb" field
the handle from the session must also be used for subsequent requests
fixes #382

@pstadler
Copy link

pstadler commented Apr 21, 2023

I thought I'd be a nice neighbor and let you know that certain regions (probably EU) seem to get redirected to a consent page during the initial call, thus failing to obtain a session. We're currently investigating possible solutions:
pstadler/ticker.sh#33 (comment)

@ethanbsmith
Copy link
Contributor Author

thx @pstadler. it looks like the cookies and crumbs are no longer needed again for v7 (at least for now). r u seeing the redirects on calls to the v7 api w/out simulating a session?

@pstadler
Copy link

it‘s happening over at my project using something very similar to your fix. Going to remove it again if it‘s working again without crumb. Wondering for how long though, it‘s been flaky for a while.

@pstadler
Copy link

I gladly reverted the change on my end. Fingers crossed that it won't happen again.

@ethanbsmith
Copy link
Contributor Author

im currently testing an update that caches the yahoo handle. i dont know what the error will ben when the handle becomes invalid, as this is probably driven by a timeout on the yahoo server side, so i cant force the error condition. will push once i have that part worked out

@pstadler
Copy link

pstadler commented May 6, 2023

my assumption is that you‘ll get a 401 Unauthorized if the session expires (if this is what you’re referring to). In this case I‘d simply try to obtain a new session and fetch the quotes again. See /~https://github.com/pstadler/ticker.sh/blob/54b73a3f57432b5efd102754c48f20ed246f8ed2/ticker.sh#L53

added handle caching across calls
@ethanbsmith

This comment was marked as outdated.

if ((r$status_code == 200) && (length(r$content) > 0)) {
ses$crumb = rawToChar(r$content)
} else {
if (!force.new) ses <- .yahooSession(TRUE) else stop("Unable to get yahoo crumb")
Copy link
Owner

@joshuaulrich joshuaulrich Jun 13, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This seems backward. I expect to call .yahooSession(TRUE) when force.new is TRUE, else throw an error because we couldn't get a crumb.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

its really a state machine. the force.new is never intended to be used directly, only in the recursive call. i can rewrite it as a loop to clean up the signature

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Calling the function recursively is fine. The logic on this line (51) confused me, but seems correct once I walk through it.

Assume force.new is FALSE. Line 45 checks that we have an active session. Line 51 checks if we're able to get a crumb. If we can't get a crumb, then !force.new is TRUE and we call .yahooSession(TRUE). So that seems like it does what it should.

I think some comments would help me remember what this is doing.

# we were unable to get a crumb
if (force.new) {
    # we couldn't get a crumb with a new session
    stop("unable to get yahoo crumb")
} else {
    # we tried to re-use a session but couldn't get a crumb
    # try to get a crumb using a new session
    ses <- .yahooSession(TRUE)
}
    

@joshuaulrich joshuaulrich merged commit 8be0ffa into joshuaulrich:master Jun 14, 2023
@joshuaulrich joshuaulrich added this to the Release 0.4.23 milestone Jun 14, 2023
joshuaulrich added a commit that referenced this pull request Jun 15, 2023
The logic is correct but confused me until I walked through it
carefully. The comments should keep future me from being confused.

Rename 'force.new' to 'is.retry' to make it clearer that the
call is a 'retry' call. Clean up existing comments.

See #382. See #383.
@ethanbsmith ethanbsmith deleted the 382_add_session_getQuote branch March 17, 2024 18:55
netbsd-srcmastr pushed a commit to NetBSD/pkgsrc that referenced this pull request Jan 4, 2025
### Changes in 0.4.25 (2023-08-21)

1. Fix `getQuote.yahoo()` for API changes. Thanks to Ethan B. Smith for the
    report and patch! Also add error message for users in GDPR countries, since
    we cannot automatically consent to GDPR and the request fails without
    consent.
    [#392](joshuaulrich/quantmod#392)
    [#393](joshuaulrich/quantmod#393)
    [#395](joshuaulrich/quantmod#395)

1. Fix `getQuote.yahoo()` when the user only requested metrics that do not have
    have a value for 'regularMarketTime'. Set the value to NA in these cases
    so the output remains the same regardless of whether the endpoint returns
    a 'regularMarketTime' or not. Thanks to @mehdiMBH for the report!
    [#255](joshuaulrich/quantmod#255)

1. Add fields to `getQuote.yahoo()` that are returned when no fields are
    explicitly requested. Thanks to @Courvoisier13 for the report!
    [#335](joshuaulrich/quantmod#335)

1. Add intraday endpoint to `getSymbols.yahoo()`. Thanks to @kapsner for the
    report and patch! Also allow suppressing the warning if more than 7 days
    of data are requested (@eddelbuettel).
    [#351](joshuaulrich/quantmod#351)
    [#381](joshuaulrich/quantmod#381)
    [#399](joshuaulrich/quantmod#399)

1. Add warning if `getSymbols()` is called with tickers that are reserved words
    because accessing them requires back-quotes (e.g. ``NA``).
    [#401](joshuaulrich/quantmod#401)

1. Fix `allReturns()` when 'subset' is specified. Thanks to @Panagis1980 for
    the report!
    [#402](joshuaulrich/quantmod#402)

### Changes in 0.4.24 (2023-07-17)

1. Fix `getSymbols.oanda()` URL. Thanks to @macray76 for the report.
    [#387](joshuaulrich/quantmod#387)

### Changes in 0.4.23 (2023-06-14)

1. Fix `getQuote.yahoo()` error. Thanks to Ethan B. Smith for the report and
    patch!
    [#382](joshuaulrich/quantmod#382)
    [#383](joshuaulrich/quantmod#383)

1. Add `name` argument to `add_TA()`. Thanks to @SamoPP for the suggestion!
    [#377](joshuaulrich/quantmod#377)
    [#205](joshuaulrich/quantmod#205)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

getQuote.yahoo 401 error
3 participants