Generate error codes

[mattheworiordan]

See ably/ably-common#32

Whilst I do like this, I am concerned that a change to the wording in the errors.json file (which is reasonable to do), could now result in this library being very broken i.e. an exception that causes the library to exit.

I have added 3f8351b which may help.

One other approach is to keep a list of used constants at the top of each file so that it will break immediately when the library is loaded, but that feels a bit heavy handed. Perhaps we should do this though in untyped languages to ensure the libraries don't become brittle and fail badly at the wrong time. WDYT?

[paddybyers]

ok, so now we have two scripts that process the errors.json :(

It is the responsibility of the library to make sure it's not broken if the ably-common submodule ref is updated. If that requires a static check then yes, something ought to be done to ensure that.

[mattheworiordan]

ok, so now we have two scripts that process the errors.json :(

Sorry, I don't follow. Where is the second script?

Code generators will surely always be different for every language?

[paddybyers]

Where is the second script?

@gernest wrote one, which can output for any language based on a template, and my ably-common issue was (in part) the proposal to include that in that repo.

[gernest]

@paddybyers I think @mattheworiordan approach is okay. It feels more natural that having a go binary doing that. Each language can use the language's tools to generate the constants.

And just to clear things, my script is only generating go code for now, I only pointed out that it uses templates so it is possible to do that for other languages see ably/ably-go#72.

[paddybyers]

@gernest right, but I wanted to make sure that at least we use consistent logic for converting the error strings into constant names so a single approach, if generic enough, would be best.

[mattheworiordan]

but I wanted to make sure that at least we use consistent logic for converting the error strings into constant names

But every language has an idiomatic way of defining constants, so why would we try to centralise this in one place, and also make it impossible for other devs to maintain their libs without getting Go code / templates written?

[ORBAT]

@mattheworiordan: I think what @gernest is getting at is that it wouldn't require writing Go at all, just templates with a fairly self-explanatory syntax if you see an example. Right now the template in ably/ably-go#72 is baked in, but reading it from a file is trivial. So one centralised tool (which doesn't have to reside in the Go lib repo) for generating the error code constants for any language there's a template for. Customers don't need to be exposed to it if we keep an up-to-date version of the constants in master, and that can probably be automated

[mattheworiordan]

Customers don't need to be exposed to it if we keep an up-to-date version of the constants in master, and that can probably be automated

Well perhaps, I don't think it makes much difference to be honest. I would say that it's a lot easier this way around in that no one has to do PRs on other repos just to get their constants in, but I don't feel that strongly about it.

A data type (JSON) IMHO is far more predictable as a dependency than a templating language in another repo.

[mattheworiordan]

@paddybyers @SimonWoolf see acae470. If you are happy with this, I think I would like to raise issues in all libs to follow suit.

[mattheworiordan]

TBH I still don't really follow why we want make it the client lib's responsibility to add the url to the message (at least for errors received from realtime).

Well sure, I agree that makes sense if the only time an error generated is from realtime. Client libraries generate errors too. See

ably-ruby/lib/ably/models/message_encoders/cipher.rb

Line 31 in 2764efc

raise Ably::Exceptions::CipherError.new(e.message, nil, 92004)

for example.

so this logic only needs to cover library-generated errors which won't yet have an href

Sure, but an error code should still map to an error URL no?

[SimonWoolf]

I agree that makes sense if the only time an error generated is from realtime. Client libraries generate errors too

Yes, that's why my suggestion was to use the absence of message.href to decide whether to add to the message, as only errors from realtime, not client-lib generated errors, will have an href (and errors from realtime will already have the url as part of the message)

Sure, but an error code should still map to an error URL no?

The href field in an error realtime returns can contain whatever we like. Probably almost all of the time it will just contain https://help.ably.io/<code>.

But maybe sometimes it won't.

E.g. maybe we'll have a code that covers multiple different cases that we can't change for backwards-compatibility reasons, but we could still link to different appropriate solutions article for each case.

We have that flexibility; it seems silly to throw it away by imposing an unnecessary constraint on future-us that all urls have to be help.ably.io/<code> (which is effectively what we'd be doing if the client library uses the absence of help.ably.io in the message to decide whether to add its own message).

[mattheworiordan]

Yes, that's why my suggestion was to use the absence of message.href to decide whether to add to the message, as only errors from realtime, not client-lib generated errors, will have an href (and errors from realtime will already have the url as part of the message)

Ok, so pre 1.2, add href to the message if href attribute missing, and always make sure the href attribute is logged when present. 1.2+ no need to add to the message, but still always output the href attribute right?

The href field in an error realtime returns can contain whatever we like. Probably almost all of the time it will just contain https://help.ably.io/<code>.

Yes. Although it's https://help.ably.io/errors/<code>

But maybe sometimes it won't.

Why not? Letting client lib SDKs decide when it's appropriate or not is dangerous. I don't really follow what the downside is. We have systems in place to detect error codes that are being viewed and we don't have articles for, and we have a support system that can grow as our needs evolve. If you need to additionally include an href, there's nothing stopping you adding that into the message. But leaving this to the the client library developer's discretion, will simply result in divergence from a sec we're introducing to solve this exact problem.

E.g. maybe we'll have a code that covers multiple different cases that we can't change for backwards-compatibility reasons, but we could still link to different appropriate solutions article for each case.

Options are:

• Include an additional URL in the message
• Overide the href attribute
• Handle these variations in the support article
• Introduce a new sub-format for error codes

We have that flexibility; it seems silly to throw it away by imposing an unnecessary constraint on future-us that all urls have to be help.ably.io/<code> (which is effectively what we'd be doing if the client library uses the absence of help.ably.io in the message to decide whether to add its own message).

We're not throwing away any flexibility. We're introducing a uniform way to approach error codes that improves developer experience. Leaving this to the discretion of the developer is throwing away the opportunity to uniformly improve things. It also encourages everyone to think carefully about the error codes they use so that we build a meaningful knowledgebase of error codes and what they mean. The absence of this is what encourages people to write error messages that customers don't understand, and get in touch with us if we're lucky, or worse just leave.

[SimonWoolf]

The href field in an error realtime returns can contain whatever we like

Letting client lib SDKs decide when it's appropriate or not is dangerous.

I'm a bit confused right now -- I've no idea what thing you think I was saying that we should let client libs decide.

Lets chat it over at the standup this week?

[mattheworiordan]

I'm a bit confused right now -- I've no idea what thing you think I was saying that we should let client libs decide.

Typo, I meant client lib SDK developers decide.

I see, so you are saying that realtime (not the client libraries) reserves the right to send any href it likes right? And any client lib generated error, which has an error code too, would default to help.ably.io/errors/[ERR] unless href is overided?

[paddybyers]

I see, so you are saying that realtime (not the client libraries) reserves the right to send any href it likes right? And any client lib generated error, which has an error code too, would default to help.ably.io/errors/[ERR] unless href is overided?

Yes. Currently realtime only sends errors with a canonical href; but if an href is present, the library should not replace it.

[mattheworiordan]

One thing to bear in mind, is that people search for error codes. Sure, they can follow a link from an error message, but they may just search too, such as https://www.ably.io/support?addsearch=40101. Of course we could override the default generated help URL from realtime, or from the client lib, but we still need to remember that the error code may lead that person to a support article on that error code.

[mattheworiordan]

@paddybyers @SimonWoolf how about ea425be? Does that solve the problems of not duplicating error URLs unnecessarily, and providing flexibility to realtime or the client libs to specify their own href if they want which takes precedence over any auto-generated href? If you think that's OK, I'll update the Exception stringify logic too, push this up, and perhaps adjust the spec slightly to account for all this?

[SimonWoolf]

👍

[mattheworiordan]

@paddybyers you happy with that? If so, I am going to raise an issue in all libraries to follow suit ASAP.

[paddybyers]

LGTM, thanks