Runner shouldn't retry getUpdates on unrecoverable errors

[slavafomin]

Hello!

I'm seeing a nonoptimal behavior of runner when it tries to getUpdates multiple times when Telegram responds with a non-recoverable error: 409: Conflict: can't use getUpdates method while webhook is active; use deleteWebhook to delete the webhook first.

I would suggest to fail fast on such errors and not retry the request.

[KnorpelSenf]

This is the current behaviour with the built-in polling of grammY. It makes sense to copy over the same pattern to the grammY runner.

It may be a bit challenging because we currently do not have grammY as a dependency. This means that we don't have access to the error objects. Hehe, we cannot use the same code. However, this can probably be worked around somehow. Do you have the capacity to look into this?

[slavafomin]

Should the logic be added here? Or is it more complex than that?

[KnorpelSenf]

It can be added there. It's not hard.

[slavafomin]

Okay, I will try to create a PQ for this.

[KnorpelSenf]

The equivalent logic in the core package can found at https://github.com/grammyjs/grammY/blob/29e374acbb868a397fabc067ff9f5b4c5868bb40/src/bot.ts#L488

[slavafomin]

The equivalent logic in the core package can found at https://github.com/grammyjs/grammY/blob/29e374acbb868a397fabc067ff9f5b4c5868bb40/src/bot.ts#L488

I see it's already handled in the core package, doesn't it?

[slavafomin]

By the way, I don't see errors documented in the official bot API page. Do you know any other source for this?

[KnorpelSenf]

Okay, I will try to create a PQ for this.

Amazing, thanks!

The equivalent logic in the core package can found at https://github.com/grammyjs/grammY/blob/29e374acbb868a397fabc067ff9f5b4c5868bb40/src/bot.ts#L488

I see it's already handled in the core package, doesn't it?

That's correct. Do you mean we should simply expose fetchUpdates? That's a good idea, I think! We will need to adjust its documentation then to make sure people understand that this internal method should not be called by anyone (except in rare cases like ours).

[KnorpelSenf]

By the way, I don't see errors documented in the official bot API page. Do you know any other source for this?

I know that no such source exists, and that this is an intentional decision. Not everyone may agree with this approach, but the reasoning is that development should not be error-driven. If you write correct code, it doesn't throw errors. Hence, if you do run into errors, your code is incorrect and it needs to be fixed by the developer rather than automatically.

[slavafomin]

Okay, I will try to create a PQ for this.

Amazing, thanks!

The equivalent logic in the core package can found at https://github.com/grammyjs/grammY/blob/29e374acbb868a397fabc067ff9f5b4c5868bb40/src/bot.ts#L488

I see it's already handled in the core package, doesn't it?

That's correct. Do you mean we should simply expose fetchUpdates? That's a good idea, I think! We will need to adjust its documentation then to make sure people understand that this internal method should not be called by anyone (except in rare cases like ours).

Actually I'm not sure that we should do this. Exporting some internal function from the public package look like a bad idea. The proper way would be to extract this logic into a separate utility package, but I'm not sure if it worth it in this case. Maybe we should keep it simple an just have two copies of this logic for now? I guess it doesn't look critical or bulky enough to extract it.

[KnorpelSenf]

I agree with this.

[KnorpelSenf]

Completed with #8.