Eula error handling #1166

ana-sher · 2025-12-10T22:47:51Z

Resolves #36
Contains:

@betolink unit test that is reproducing EULA error (comes in response.text, with html that includes "eula" error inside)
fix in _download_file, because if catching error further it won't contain text or content, will only raise error for given status code.

Pull Request (PR) draft checklist - click to expand

Please review our
contributing documentation
before getting started.
Populate a descriptive title. For example, instead of "Updated README.md", use a
title such as "Add testing details to the contributor section of the README".
Example PRs: #763
Populate the body of the pull request with:
- A clear description of the change you are proposing.
- Links to any issues resolved by this PR with text in the PR description, for
  example closes #1. See
  GitHub docs - Linking a pull request to an issue.
Update CHANGELOG.md with details about your change in a section titled
## Unreleased. If such a section does not exist, please create one. Follow
Common Changelog for your additions.
Example PRs: #763
Update the documentation and/or the README.md with details of changes to the
earthaccess interface, if any. Consider new environment variables, function names,
decorators, etc.

Click the "Ready for review" button at the bottom of the "Conversation" tab in GitHub
once these requirements are fulfilled. Don't worry if you see any test failures in
GitHub at this point!

Pull Request (PR) merge checklist - click to expand

Please do your best to complete these requirements! If you need help with any of these
requirements, you can ping the @nsidc/earthaccess-support team in a comment and we
will help you out!

Add unit tests for any new features.
Apply formatting and linting autofixes. You can add a GitHub comment in this Pull
Request containing "pre-commit.ci autofix" to automate this.
Ensure all automated PR checks (seen at the bottom of the "conversation" tab) pass.
Get at least one approving review.

📚 Documentation preview 📚: https://earthaccess--1166.org.readthedocs.build/en/1166/

github-actions · 2025-12-10T22:48:03Z

👈 Launch a binder notebook on this branch for commit 48eacfa

I will automatically update this comment whenever this PR is modified

👈 Launch a binder notebook on this branch for commit cdb5135

👈 Launch a binder notebook on this branch for commit 2f8f4d2

👈 Launch a binder notebook on this branch for commit ad3a3ff

👈 Launch a binder notebook on this branch for commit 9631a72

github-actions · 2025-12-10T22:48:16Z

User ana-sher does not have permission to run integration tests. A maintainer must perform a security review of the code changes in this pull request and re-run the failed integration tests jobs, if the code is deemed safe.

chuckwondo

Thanks @ana-sher for picking this up!

chuckwondo · 2025-12-11T15:54:10Z

earthaccess/exceptions.py

    pass
+
+
+class EulaException(Exception):


@betolink, what do you think about making this a subclass of DownloadFailure?

Maybe? one thing I forgot is that this error will also arise when we open the file with .open() would that be semactically download error too? @chuckwondo

I'd say that it would be semantically the same. When opening, we still end up attempting to "download" data. The only difference is how much data is being downloaded (either all of it, or some of it).

In the case of open, a user likely won't care or notice whether or not EulaException is a DownloadFailure, but in the case of download, it might be nice to be able to catch DownloadFailure and have that also catch EulaException.

Also, now that I think about it, I suggest we rename EulaException to EulaNotAccepted to make it a bit more clear (note also the intentional lack of the suffix Error or Exception).

If we are having same issue on .open(), will try to reproduce and add test on that as well, thank you @betolink.
Might need to intercept and raise in _open_urls_https or _open_files.

testing:
when calling earthaccess.open for

"S5P_L2__CH4____HiR" file-like objects were created (maybe further xarray processing would give error).

"sentinel-1c_raw" 401 returned that was wrapped in FileNotFoundError for the file by fsspec. By opening that file url in browser - automatically redirected me to accept eula, so giving data url if fsspec returns FileNotFoundError might be a good idea. But even fsspec returns it:

raise FileNotFoundError(url) from exc E FileNotFoundError: https://datapool.asf.alaska.edu/RAW/SC/S1C_IW_RAW__0SDV_20250328T182304_20250328T182337_001645_002A8D_2E72.zip C:\Users\dmitr\miniconda3\envs\earthaccess\Lib\site-packages\fsspec\implementations\http.py:451: FileNotFoundError

"ALOS2_L1_PSR2" (also has eula 403 on download) file-like objects were created without error.

"ERS-1_L1" - FileNotFoundError

"ALOS_PSR_RTC_LOW" - FileNotFoundError

Suggesting to move earthaccess.open EULA error processing to another PR since we are not sure yet how to infer it from returned FileNotFoundError

chuckwondo · 2025-12-11T16:42:57Z

earthaccess/store.py

+                if r.status_code in [401, 403]:
+                    text = (r.text or "").lower()
+                    if "eula" in text:
+                        raise EulaException(
+                            "You must accept the EULA to download this data."
+                        )
+
                r.raise_for_status()


If the response text contains "eula" (ignoring case), we should use that text as the error message, rather than making up our own message.

Also, if the response is not successful, but the text does not include "eula", we want to raise a general DownloadFailure (which seems to have been accidentally left out as part of the previous PR that added the DownloadFailure exception class -- i.e., the class was added, but wasn't used here, as it should have been).

Suggested change

if r.status_code in [401, 403]:

text = (r.text or "").lower()

if "eula" in text:

raise EulaException(

"You must accept the EULA to download this data."

)

r.raise_for_status()

if not r:

if "eula" in (text := r.text or "").lower()

raise EulaException(text)

raise DownloadFailure(text)

We want to include full html page that was returned in text to exception message? I could strip it down to the closest ">" message "<" if we are sure it's always HTML.

Oh, right. I forgot it's returning HTML.

I don't think it's worth trying to parse out the message at runtime. Can you reproduce the response and just manually copy the text of the message and then paste it into the code?

what about logging the content and telling the users that the full response is in the error logs?

I'm leaning towards not logging the full response, because it is a bunch of HTML, and would likely be annoying and probably too verbose and messy for a user to bother to read to find the error message.

If we can reproduce the HTML response without much effort, I'd say we should do that (like you did during our hack session the other day), find the error message in the HTML response, and copy the error message to put into the code. We may want to slightly adjust the wording, and include the data URL in the message too, so the user knows exactly what failed to be downloaded.

Was reproducing this issue again and got 403 instead of 401 with json, not sure why. Was testing for S5P_L2__CH4____HiR (same collection as for hackdays) and same credentials. Also tested for sentinel-1c_raw and ALOS2_L1_PSR2 - same json with eula error was returned. For ALOS_PSR_RTC_LOW and ERS-1_L1 got 401 with text HTTP Basic: Access denied, no EULA mentioned...

I can check content-type in headers and output error_description + resolution_url + data url if json. If not - fallback to "Eula Acceptance Failure" + data url?

chuckwondo · 2025-12-11T16:45:00Z

tests/unit/test_store.py

+        )
+        store = Store(self.auth)
+        # test
+        with self.assertRaises(EulaException):


Suggested change

with self.assertRaises(EulaException):

with self.assertRaises(EulaException, msg=response):

tests/unit/test_store.py

Co-authored-by: Chuck Daniels <[email protected]>

github-actions · 2025-12-11T21:29:59Z

User ana-sher does not have permission to run integration tests. A maintainer must perform a security review of the code changes in this pull request and re-run the failed integration tests jobs, if the code is deemed safe.

chuckwondo · 2025-12-11T21:44:18Z

@ana-sher, looks like pre-commit failed on one of my suggestions: https://results.pre-commit.ci/run/github/399867529/1765488551.Ty6xULlSTtagswoCnbUJyQ

github-actions · 2025-12-16T16:47:25Z

User ana-sher does not have permission to run integration tests. A maintainer must perform a security review of the code changes in this pull request and re-run the failed integration tests jobs, if the code is deemed safe.

github-actions · 2026-01-05T22:40:00Z

User ana-sher does not have permission to run integration tests. A maintainer must perform a security review of the code changes in this pull request and re-run the failed integration tests jobs, if the code is deemed safe.

github-actions · 2026-01-19T20:31:10Z

User ana-sher does not have permission to run integration tests. A maintainer must perform a security review of the code changes in this pull request and re-run the failed integration tests jobs, if the code is deemed safe.

github-actions · 2026-01-19T21:21:16Z

User ana-sher does not have permission to run integration tests. A maintainer must perform a security review of the code changes in this pull request and re-run the failed integration tests jobs, if the code is deemed safe.

chuckwondo

Looks great. Thanks @ana-sher!

chuckwondo · 2026-01-20T19:37:38Z

@betolink, @ana-sher and I just went over this again in today's hackday, and I think it looks good. I approved it, but wanted you to take a quick look (there's not much code to review) before merging, especially since we started this with you on a previous hackday a couple months ago.

betolink

Looks good to me! thanks for the PR @ana-sher!

betolink and others added 2 commits December 9, 2025 13:46

add failing unit test for EULA exceptions

d55703b

Add eula check in 401, 403 response

48eacfa

ana-sher marked this pull request as ready for review December 10, 2025 23:34

chuckwondo requested changes Dec 11, 2025

View reviewed changes

Apply suggestions from code review

cdb5135

Co-authored-by: Chuck Daniels <[email protected]>

betolink assigned ana-sher Dec 12, 2025

Rename eula error, add download error, fix ruff lint

2f8f4d2

Merge branch 'main' into eula-error-handling

ad3a3ff

Add checking on exact error message, add one more test

9631a72

ana-sher requested a review from chuckwondo January 19, 2026 21:39

chuckwondo approved these changes Jan 20, 2026

View reviewed changes

betolink approved these changes Jan 28, 2026

View reviewed changes

betolink merged commit 80ba1e0 into nsidc:main Jan 28, 2026
11 of 14 checks passed

	with self.assertRaises(EulaException):
	with self.assertRaises(EulaException, msg=response):

Eula error handling #1166

Eula error handling #1166

Uh oh!

Conversation

ana-sher commented Dec 10, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

github-actions bot commented Dec 10, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

github-actions bot commented Dec 10, 2025

Uh oh!

chuckwondo left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

chuckwondo Dec 12, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

github-actions bot commented Dec 11, 2025

Uh oh!

chuckwondo commented Dec 11, 2025

Uh oh!

github-actions bot commented Dec 16, 2025

Uh oh!

github-actions bot commented Jan 5, 2026

Uh oh!

github-actions bot commented Jan 19, 2026

Uh oh!

github-actions bot commented Jan 19, 2026

Uh oh!

chuckwondo left a comment

Choose a reason for hiding this comment

Uh oh!

chuckwondo commented Jan 20, 2026

Uh oh!

betolink left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

ana-sher commented Dec 10, 2025 •

edited

Loading

github-actions bot commented Dec 10, 2025 •

edited

Loading

chuckwondo Dec 12, 2025 •

edited

Loading