alastairp, ruaok: regarding a separate blueprint for social features, do we consider similar-users part of core blueprint, social or different?
2021-03-19 07852, 2021
alastairp
🍅 🍅
2021-03-19 07855, 2021
Cyna[m]
Yea I have some idea of minified notifications but need to confirm the list of notification types with reo first. If the list is small. I can make a minified notification cause showing a mail in small notification bar doesnt make sense
2021-03-19 07809, 2021
alastairp
_lucifer: API blueprint?
2021-03-19 07811, 2021
Cyna[m]
Might need to have different template for site baed notifications
regarding logging level, `error` instead of critical like we do elsewhere makes sense to me.
2021-03-19 07803, 2021
akashgp09 has quit
2021-03-19 07815, 2021
alastairp
yes, this is another thing that we've tried a bunch of ideas for, but never come to a clear guideline on what to catch
2021-03-19 07827, 2021
alastairp
in lots of places we don't catch postgres errors, for example, and maybe we should
2021-03-19 07851, 2021
alastairp
what I'm trying to avoid with this comment is that we copy existing code, and then some random failure happens and sentry emails us
2021-03-19 07825, 2021
alastairp
we want to try and minimise the sentry emails, not increase them, otherwise they turn into noise and we can't find things to fix
2021-03-19 07810, 2021
alastairp
if logger.error sends a sentry email (I'm not 100% sure of this) then I don't want to receive it. Perhaps our logging setup is wrong, and we shouldn't integrate text logging and sentry error reporting with the same system
alastairp, yeah that makes sense. We even have a `log_raise_400` method but only use it sparingly.
2021-03-19 07850, 2021
alastairp
that's an even worse method - we removed a number of places where we called it
2021-03-19 07812, 2021
_lucifer
oh
2021-03-19 07822, 2021
alastairp
because we raise http400 when a user provides us with bad data, so it means that _we_ get a notification every time a user sends bad data to the API
2021-03-19 07828, 2021
_lucifer
yeah, right. :(
2021-03-19 07821, 2021
alastairp
I had ideas of using sentry as a general looger - for example being able to use it to track messages about how a process was running. but it never worked out very well
2021-03-19 07829, 2021
adhawkins_ joined the channel
2021-03-19 07855, 2021
adhawkins has quit
2021-03-19 07858, 2021
alastairp
at the time we wanted to get information about the execution of the recommendations, for example, and the options were to log to file (hard to read in the context of spark) or use sentry, so we decided to use sentry
so I've been trying to think about different ways of using sentry - just keep it as an error reporter. anything that appears on sentry is a bug in our code that we should improve
2021-03-19 07840, 2021
adhawkins_ is now known as adhawkins
2021-03-19 07859, 2021
_lucifer
yeah that makes sense to me
2021-03-19 07811, 2021
_lucifer
>To summarize, Sentry works with your application logging infrastructure, often integrating directly. It does not replace the need for those logs, and it's also not a destination for things that aren't actionable errors or crashes.
2021-03-19 07814, 2021
alastairp
so this is why I'm unsure about the "try/except Exception: log to sentry" workflow
2021-03-19 07856, 2021
alastairp
I can see why we do it - it means that the user gets a nice "there was an error" message, and we still get the log information
2021-03-19 07859, 2021
alastairp
however, I'm thinking out loud - APIInternalServerError is still just a json error message
2021-03-19 07841, 2021
_lucifer
yeah, i think we would want InternalServerError. BadRequest not so much
2021-03-19 07843, 2021
alastairp
we get json error messages in https://github.com/metabrainz/listenbrainz-server… anyway, so we could just let it fail. sentry will log a message for us anyway, and the user will still get a json error message
2021-03-19 07820, 2021
alastairp
you're right about what sentry says that it does, but we have to consider that in the wider context of what resources we have available at MeB
2021-03-19 07803, 2021
alastairp
we _don't_ have a global log collecting service. that'd be another tool to run. so we then need to decide are we happy to ssh into servers and grep logs, or do we try and make sentry help us a little bit
2021-03-19 07809, 2021
_lucifer
yeah, thats true.
2021-03-19 07822, 2021
alastairp
I had hoped that we could use sentry, but I'm happy to admit that the experiment wasn't very successful
2021-03-19 07803, 2021
_lucifer
alastairp, for this PR, i'll remove the Exception clause and let it fail. let's open a ticker for what to do in the general context and discuss again later?
2021-03-19 07827, 2021
Cyna[m] uploaded an image: (1359KiB) < https://matrix.org/_matrix/media/r0/download/matrix.org/evkdOOMJBEDtBgrnqIlJljCB/image.png >
2021-03-19 07831, 2021
Cyna[m]
Mr_Monkey: This is an example for icon
2021-03-19 07855, 2021
Cyna[m]
We obviously can use a better icon. This is just something I picked from google images.
2021-03-19 07809, 2021
_lucifer
alastairp: also, as a hack we could probably change the error hirerachy here https://github.com/metabrainz/listenbrainz-server… . Make `APIError` not inherit from `Exception` or create a separate base error for exceptions that should be treated as an Exception and captured by sentry.
2021-03-19 07828, 2021
_lucifer
would that be helpful at least in the short run while we come up with a better solution?
2021-03-19 07849, 2021
Mr_Monkey
Yeah, that's about the type of thing I would imagine
This is all the ones I could find while reading the code
2021-03-19 07808, 2021
RikkoM has quit
2021-03-19 07808, 2021
RikkoM joined the channel
2021-03-19 07857, 2021
blup
wait 200 tracks is to much for a play list? since when ? :D
2021-03-19 07810, 2021
blup
wait why am I blup?
2021-03-19 07822, 2021
blup
man, my connection must be frazzled
2021-03-19 07812, 2021
blup is now known as CatQuest
2021-03-19 07801, 2021
ruaok
Social, i think. But I don't feel strongly about it.
2021-03-19 07852, 2021
alastairp
_lucifer: oh, there's some extra flask magic here
2021-03-19 07816, 2021
alastairp
1) exceptions should always inherit from Exception
2021-03-19 07820, 2021
alastairp
2) you can do a flask error handler for a specific exception subclass (e.g. APIError), and flask will use the handler whenever this exception is raised. this is how we make errors in the API get returned in json (we have a handler for APIError that uses jsonify)
2021-03-19 07843, 2021
alastairp
what this doesn't cover though is programming errors that cause exceptions (ValueError, AttributeError, requests download error, postgres exec error). what we found is that any programming error that caused an exception would result in flask's default error handler being called, which returns html, even in the API
2021-03-19 07844, 2021
alastairp
it's likely that this pattern of catching Exception and returning an APIBadRequest was a way of forcing error messages to come out of the API as json
2021-03-19 07824, 2021
alastairp
however, this is what PR 1235 fixes. now any unhandled exception that happens while the URL starts with API_PREFIX will get converted into json no matter what the error is
