Bitbucket Cloud: provide custom raise_for_status

[flichtenheld]

The default one in AtlassianRestAPI does not work
for Bitbucket Cloud.

Signed-off-by: Frank Lichtenheld frank@lichtenheld.com

[flichtenheld]

Found this one while trying to make #904 work for Bitbucket Cloud.

[slmnsh]

It'll be better if we just revert 50b5c71 instead of adding error handling one by one

[flichtenheld]

It'll be better if we just revert 50b5c71 instead of adding error handling one by one

What was the behavior before? To me it looks like before it just used the HTTP error message which is useless in many cases. It is definitely better to show the actual error message from the JSON response. But it seems that the JSON format is different for each service...

[slmnsh]

It'll be better if we just revert 50b5c71 instead of adding error handling one by one

What was the behavior before? To me it looks like before it just used the HTTP error message which is useless in many cases. It is definitely better to show the actual error message from the JSON response. But it seems that the JSON format is different for each service...

If you look the tagged commit carefully you'll see it was showing proper error message from json data. Also it has different error handling for each service.

[flichtenheld]

It'll be better if we just revert 50b5c71 instead of adding error handling one by one

What was the behavior before? To me it looks like before it just used the HTTP error message which is useless in many cases. It is definitely better to show the actual error message from the JSON response. But it seems that the JSON format is different for each service...

If you look the tagged commit carefully you'll see it was showing proper error message from json data. Also it has different error handling for each service.

That might be true for some services, but that commit did not remove any error handling from atlassian/bitbucket. I just verified that. Without my patch and with the referenced commit reverted you get something like (for a random error that I hit today):

requests.exceptions.HTTPError: 403 Client Error: Forbidden for url: https://api.bitbucket.org/2.0/repositories/openvpntechnologies/ovpn3-build/commit/4e857d6c9df6956932ed0b25abb8454892dcebea/statuses/build

With my patch you get:

requests.exceptions.HTTPError: Authenticated user is neither the creator (frank_lichtenheld) of this build status nor an admin of this repository.

So I think this is a definite improvement for Bitbucket Cloud specifically.

[slmnsh]

It'll be better if we just revert 50b5c71 instead of adding error handling one by one

What was the behavior before? To me it looks like before it just used the HTTP error message which is useless in many cases. It is definitely better to show the actual error message from the JSON response. But it seems that the JSON format is different for each service...

If you look the tagged commit carefully you'll see it was showing proper error message from json data. Also it has different error handling for each service.

That might be true for some services, but that commit did not remove any error handling from atlassian/bitbucket. I just verified that. Without my patch and with the referenced commit reverted you get something like (for a random error that I hit today):

requests.exceptions.HTTPError: 403 Client Error: Forbidden for url: https://api.bitbucket.org/2.0/repositories/openvpntechnologies/ovpn3-build/commit/4e857d6c9df6956932ed0b25abb8454892dcebea/statuses/build

With my patch you get:

requests.exceptions.HTTPError: Authenticated user is neither the creator (frank_lichtenheld) of this build status nor an admin of this repository.

So I think this is a definite improvement for Bitbucket Cloud specifically.

How about reverting that commit and keeping your patch too?

[flichtenheld]

How about reverting that commit and keeping your patch too?

I don't have a specific opinion on that. But I will definitely not make that part of my PR.

[gonchik]

@salmannotkhan does it really need to be revert it ?
I dig into code, it was to quite duplication.
I will be happy to understand the exact usage

[slmnsh]

@salmannotkhan does it really need to be revert it ?
I dig into code, it was to quite duplication.
I will be happy to understand the exact usage

Okay, let me explain what happened.

I'm using this library for 6 months never faced any issues. But one day I was making a python script to update labels on every bitbucket that I have.

I didn't know that repo labels can't contain uppercase characters. So I tried to set labels that are uppercase but instead of returning the error from API saying that "Labels can't contain the uppercase letter" I got a KeyError: "errorMessage" key doesn't exist on d or Something like that.

That means error messages aren't handled properly after the try to handle all errors messages singly because each service has its error message format.

[gonchik]

@salmannotkhan noted thank you. My bad in that situation.
Is it ok if start only for BB cloud, because I use for Jira, Confluence on-prem and error handling works well?
Or I use a little bit in other way then other developers

[slmnsh]

@salmannotkhan noted thank you. My bad in that situation.
Is it ok if start only for BB cloud, because I use for Jira, Confluence on-prem and error handling works well?
Or I use a little bit in other way then other developers

Yup only BB cloud will be fine for now because that is only causing errors for me

[gonchik]

@salmannotkhan let me upload a new release.
Too bad to read once someone meet errors

[slmnsh]

@salmannotkhan let me upload a new release.
Too bad to read once someone meet errors

No problem, sir it's a great library for atlassian services

[clrusby]

@gonchik - given it does affect other clients (eg bitbucket server), could we amend your commit 50b5c71 so it reads something like this instead?

error_msg = "\n".join(j.get("errorMessages", []) + [k + ": " + v for k, v in j.get("errors", {}).items()])

[gonchik]

@clrusby sure!

[clrusby]

What I wrote doesn’t actually work for bitbucket server as errors is a list of dict.
It does get a bit messy to handle it generically here, which makes me wonder whether it’s better to add a bespoke error handler for bitbucket server.