balena-io / balena-sdk

The SDK to make balena powered JavaScript applications.
https://www.balena.io/
Apache License 2.0
145 stars 46 forks source link

Should use the response message from failed device action requests #577

Open thgreasi opened 5 years ago

thgreasi commented 5 years ago

Atm we catch the 404 error and regardless of the response body we change the error to a 'device not found' error. We should use the response body (if available) now that the API sends mode descriptive responses.

HQ: https://github.com/resin-io/hq/issues/1293 See: https://github.com/resin-io/resin-sdk/blob/e0406c2dc010482c3900dc86ba6fa6da5045726e/lib/models/device.coffee#L1010 See: https://github.com/resin-io/resin-sdk/blob/e0406c2dc010482c3900dc86ba6fa6da5045726e/lib/util/index.coffee#L97-L101

pimterry commented 5 years ago

@thgreasi I remember this from ages back, I thought we were going to change it in the API? Really it shouldn't be returning 404s at all, this is a simple API bug, right?

If the device genuinely doesn't exist, the API should return 404, so that's ok. If it does but we can't talk to it, we should return 502 or 504 (depending on whether we're getting bad data, or we can't timeout trying to talk to it). If the supervisor is returning some error because it can't do the action, it should be returning something sensible and descriptive itself, and the API should forward it.

I think if the API is returning a 404, the SDK really should be treating that as the device being missing, and if that's not what the API means then that needs fixing.

There might be work here if it's possible for the supervisor to also return 404s, and we need to interpret which kind of thing is missing, but that's pretty niche, I'm not 100% sure if it's possible, and it doesn't sound like that's the main problem.