-
Notifications
You must be signed in to change notification settings - Fork 29
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 handling of exceptions from AWS/S3 #1643
Conversation
.. otherwise when processing device types and hitting private DTs, openBalena generates lots of noise/exceptions in the logs * also correct systemd unit Co-authored-by: Josh Bowling <45343541+joshbwlng@users.noreply.github.com> change-type: patch
@@ -2,6 +2,7 @@ | |||
Description=open-balena-api | |||
Requires=confd.service | |||
After=confd.service | |||
StartLimitIntervalSec=0 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Just leaving a note here about the change. Apparently StartLimitIntervalSec
is in fact a unit option and not a service option:
return await req.promise(); | ||
} catch (err) { | ||
// catch errors for private device types when running unauthenticated | ||
logUnauthenticatedWarning(s3Client, s3Path, err); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@ab77 This atm silences ALL errors. Shouldn't we re-throw the error when this is a different type of error that logUnauthenticatedWarning didn't handle?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fair point, we should make sure other errors aren't ignored
.. required (ideally) by balena-io/open-balena#141
We have two contracts repos, one is private and the other public.
So open-balena-api can't access the private repo, to check which device types are private, but it can enumerate the entire images bucket, which has with public
List
ACL enabled. Since it can see private devices in the enumerated list, the API will try to access various assets, but will get back a 403 from S3 API, since the objects themselves don't have public-read enabled. This behaviour produces a lot of exceptions in the API logs and this is what this PR attempts to make better.A more complete solution would probably require rethinking private/public DT handling in open-balena-api vs. balena-api.
.. in any case, events now reflected in the logs as follows: