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

Better error handling and logging for Apify proxy configuration #262

Closed
HonzaTuron opened this issue Nov 30, 2023 · 4 comments · Fixed by #272
Closed

Better error handling and logging for Apify proxy configuration #262

HonzaTuron opened this issue Nov 30, 2023 · 4 comments · Fixed by #272
Assignees
Labels
high priority Do this ASAP! This is for mission-critical work or work that blocks other teams in their work. t-tooling Issues with this label are in the ownership of the tooling team.

Comments

@HonzaTuron
Copy link
Contributor

HonzaTuron commented Nov 30, 2023

Feature

We want to change the behavior of the instance of Actor.createProxyConfiguration

Current behavior:

  • throws an error if a user is not logged in (doesn't provide the proxy password)
  • throws an error if the user doesn't have access to the Apify proxy

Suggested change:
We would like to just print a warning to the console instead (or create some safer createProxyConfiguration) on the local environment. The Apify proxy just wouldn't be used at all, the configuration would do nothing.

Also, we proposed adding an additional warning - if the user has access to the Apify proxy but can't use it locally (this must be explained properly - free plan users).

We also propose an improved copy for the original error.

  • It must be clear from the first error user can simply use CLI apify login cmd to get the proxy password.

Motivation

Make Apify templates as seamless as possible. Now, a developer has to think about where it can be run and catch different errors. It is not clear from the error what to do and we have feedback from real users that struggled with this for quite some time.

Ideal solution or implementation, and any additional constraints

Make Actor.createProxyConfiguration with warnings instead of error for local environment.

Alternative solutions or implementations

No response

Other context

No response

@jbartadev jbartadev changed the title Better logging for proxy setup Better logging for Apify proxy configuration Nov 30, 2023
@jbartadev
Copy link
Member

CC @mtrunkat @B4nan

@jbartadev jbartadev changed the title Better logging for Apify proxy configuration Better error handling and logging for Apify proxy configuration Nov 30, 2023
@B4nan
Copy link
Member

B4nan commented Dec 7, 2023

I'd say we generally want users to use proxy even locally, so they have the same setup as after the deployment. Agreed we could just print a warning instead of failing locally (= when APIFY_IS_AT_HOME is not set), saying how to enable them to get rid of the warning (we could even have a guide for this and link it from the warning).

@mtrunkat mtrunkat added the high priority Do this ASAP! This is for mission-critical work or work that blocks other teams in their work. label Dec 8, 2023
@mtrunkat
Copy link
Member

Yes, ideally, everybody should use a proxy to have consistent behavior when running locally and when on the platform. But this leads to every new user who starts developing locally getting 2 errors:

  1. Access denied ➡️ you need to sign in
  2. Cannot use Apify Proxy local as on the free account

Instead, we should optimize for a quick win in the onboarding flow, and new users should be able to run every template (if possible) without signing in and having proxy access.

So:

  • If user is signed in and has external access, then use a proxy
  • If not signed or without access, then skip proxy usage, but clearly and visibly state this in the log

@B4nan could you please prioritize this? There is a large potential as, at the moment every new user gets and error and then another error. So we believe this may largely improve onboarding.

@B4nan B4nan added this to the 78th sprint - Tooling team milestone Dec 11, 2023
@B4nan B4nan added the t-tooling Issues with this label are in the ownership of the tooling team. label Dec 11, 2023
@B4nan B4nan self-assigned this Dec 11, 2023
B4nan added a commit that referenced this issue Jan 8, 2024
…d is found

We use the proxy configuration in all the templates, as we want users to run behind a proxy all the times.
This PR changes the internal mechanism to gracefully ignore the proxy configuration locally when we don't
see a valid token or proxy password. This way, new users can run their actors locally without being logged in.

Closes #262
B4nan added a commit that referenced this issue Jan 8, 2024
…d is found

We use the proxy configuration in all the templates, as we want users to run behind a proxy all the times.
This PR changes the internal mechanism to gracefully ignore the proxy configuration locally when we don't
see a valid token or proxy password. This way, new users can run their actors locally without being logged in.

Closes #262
B4nan added a commit that referenced this issue Jan 8, 2024
…d is found

We use the proxy configuration in all the templates, as we want users to run behind a proxy all the times.
This PR changes the internal mechanism to gracefully ignore the proxy configuration locally when we don't
see a valid token or proxy password. This way, new users can run their actors locally without being logged in.

Closes #262
@B4nan B4nan closed this as completed in #272 Jan 8, 2024
B4nan added a commit that referenced this issue Jan 8, 2024
…d is found (#272)

We use the proxy configuration in all the templates, as we want users to run behind a proxy all the times.
This PR changes the internal mechanism to gracefully ignore the proxy configuration locally when we don't
see a valid token or proxy password. This way, new users can run their actors locally without being logged in.

Closes #262
@B4nan
Copy link
Member

B4nan commented Jan 8, 2024

Released as v3.1.15, tried to use a fresh playwright template and it seems to be working fine.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
high priority Do this ASAP! This is for mission-critical work or work that blocks other teams in their work. t-tooling Issues with this label are in the ownership of the tooling team.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants