You should not insert new entries in the middle of enumerations?

[kendallb]

Ok, anyone who uses these API definitions to generate API code in strongly typed languages like C# or Java will end up with an enumeration for the 'error_code' field. We had a breakage yesterday when some of the carriers started returning new error values that were not defined in the spec so the resulting payload would fail to parse.

We reached out to support and the 'fix' is this commit:

e3e74cb

Which just adds the new errors to the enumeration. But that does not help anyone who has pre-compiled libraries as we all need to rebuild against the new spec. But worse, since you just inserted new values into the error table in the middle of the table, any existing binary values that represent those values when generated by an API will no longer be compatible.

Honestly it's just not a good idea to add new fields to enumerations as defined in the spec as you will break folks code running out in the wild.

[AnneOReilly]

Hey @kendallb - thanks for the feedback! It's important to be able to add items onto enumerations like this, because there are error cases which do not fit into the current list. This update was to account for various error codes that have been added over time that did not make it into the spec.

We can update this spec to have the new values at the end of the enumeration - just want to confirm that you haven't rebuilt yet?

[kendallb]

No, we have not rebuilt it yet and I have been using my own version of the API spec for years anyway as I fixed a ton of stuff that never got accepted upstream (I understand your API is auto generated). So to do it I would either need to hack up my own API spec and public new NuGet libraries based on that, or do a full merge from any changes you guys made recently.

But bear in mind someone already updated your official library with the new enumeration value (but only the pesky no_rates_returned one that caused everyone's outages last night and this morning) as well:

ShipEngine/shipengine-dotnet@1ce6116

And it was inserted in the middle as well and it is a .net library. Best hope none of your customers are using that API and storing the enumeration value somewhere as a numeric value? Or have something linked to that binary that does not get recompiled ...

To be honest I think those error codes if they are intended to be in flux should probably be specified as a string field, not an enumeration of known values as an AllOf field because that means as soon as a new value shows up in the wild unless you have recompiled your code, it WILL crash.

And we are not the only ones as there was a known 'outage' on your incident response page specifically about this so it affected enough customers for it to be declared an incident.

https://status.shipengine.com/#past-incidents

[kendallb]

BTW we worked around the issue by adding some improved exception handling to simply log the errors (we had missed a couple of spots). So we don't really need the new enum values since no rates is a critical error for that carrier anyway.