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

Worker: no error handling on fetch:credential #600

Closed
josephjclark opened this issue Feb 14, 2024 · 3 comments · Fixed by #601
Closed

Worker: no error handling on fetch:credential #600

josephjclark opened this issue Feb 14, 2024 · 3 comments · Fixed by #601

Comments

@josephjclark
Copy link
Collaborator

Just like #590, the Worker doesn't have very good error handling around fetch:credential. If for some reason a credential can't be returned (like with this Oath bug we're seeing), only bad things will happen.

At the time of writing that's manifesting as the Run being marked as Lost.

I am a bit concerned that the worker will get the run:error event before the run:start event. I think this is legit - we can't even start the run if we can't load the credentials.

(later, credentials and dataclips will be loaded on demand, rather than preloaded, so that becomes kinda moot)

This is all closely related to the dataclip stuff - see #595

@github-project-automation github-project-automation bot moved this to New Issues in v2 Feb 14, 2024
@josephjclark
Copy link
Collaborator Author

I actually think the 1-0 branch is handling this a lot better because it adds error handling to the underlying helper function.

@josephjclark
Copy link
Collaborator Author

Yes, in fact, I think in 1.0 the credential will throw or timeout, which will trigger a run:error. The error message might be cryptic (or might not!) but I think it'll hold up and the run will error without being Lost

@josephjclark
Copy link
Collaborator Author

I think the bigger question is: what error reason do we return if a credential failed to load?

If it's because the credential is hosted externally and is out of date or invalid, that feels like a crash to me.

@josephjclark josephjclark moved this from New Issues to In progress in v2 Feb 14, 2024
@github-project-automation github-project-automation bot moved this from In progress to Done in v2 Feb 15, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Archived in project
Development

Successfully merging a pull request may close this issue.

1 participant