Skip to content

Commit

Permalink
proxy: fix segfault, improve error messages
Browse files Browse the repository at this point in the history
Give a bit more details in the "No service replied" error messages.
  • Loading branch information
fvennetier committed Mar 25, 2021
1 parent d8ab0a6 commit a12a8dd
Show file tree
Hide file tree
Showing 2 changed files with 17 additions and 7 deletions.
5 changes: 5 additions & 0 deletions core/internals.h
Original file line number Diff line number Diff line change
Expand Up @@ -46,6 +46,9 @@ extern "C" {
#define ON_ENUM(P,E) case P##E: return #E

#define CODE_IS_NETWORK_ERROR(C) ((C) > ERRCODE_PARAM && (C) <= CODE_NETWORK_ERROR)
/* Tell if the error is a network error and appears after
* the connection has been established. */
#define CODE_IS_ERR_AFTER_START(C) ((C) >= ERRCODE_CONN_RESET && (C) <= ERRCODE_CONN_TIMEOUT)

#define CODE_IS_OK(C) (((C) >= CODE_FINAL_OK) && ((C) < CODE_BEACON_REDIRECT))
#define CODE_IS_TEMP(C) (((C) >= CODE_TEMPORARY) && ((C) < CODE_FINAL_OK))
Expand Down Expand Up @@ -104,6 +107,8 @@ extern "C" {
period_reload = CLAMP(period_reload,2,(P)); \
tick_reload = 1

/* Some macros make assumptions about the ordering of these error codes.
* Be careful if you plan to change them. */
enum {
ERRCODE_UNKNOWN_ERROR = 0,
ERRCODE_PARAM = 1,
Expand Down
19 changes: 12 additions & 7 deletions proxy/common.c
Original file line number Diff line number Diff line change
Expand Up @@ -441,30 +441,35 @@ gridd_request_replicated (struct req_args_s *args, struct client_ctx_s *ctx,
* the subsequent requests. */
service_invalidate(url);

/* TODO(jfs): should we let the client retry or occupy a
* thread in the proxy to make all the necessary retries ? */
/* JFS: should we let the client retry or occupy a
* thread in the proxy to make all the necessary retries?
* FVE: in some cases where we are not sure the request
* actually failed, we will let the client retry. */

/* that error is not strong enough to stop the iteration, we
* just try with another service */
g_clear_error (&err);
GError *last_err = err;
err = NULL;

/* But if we expected at least one service to respond,
* and we still encounter that error with the last URL of the
* array (!pu[1]), then this is an overall error that we should return. */
if ((ctx->which != CLIENT_RUN_ALL
&& ctx->which != CLIENT_SPECIFIED) && !next_url) {
err = BUSY("No service replied");
err = BUSY("No service replied (last error: (%d) %s)",
last_err->code, last_err->message);
stop = TRUE;
} else if (ctx->which == CLIENT_PREFER_MASTER &&
(err->code == ERRCODE_CONN_CLOSED
|| err->code == ERRCODE_READ_TIMEOUT)) {
CODE_IS_ERR_AFTER_START(last_err->code)) {
/* Maybe the request is running in the background.
* For requests on master, let the client decide to try again.
* Retrying may trigger an error (such as a conflict),
* if the request has already been executed. */
err = BUSY("No service replied");
err = BUSY("No service replied (last error: (%d) %s)",
last_err->code, last_err->message);
stop = TRUE;
}
g_clear_error(&last_err);
} else if (CODE_IS_RETRY(err->code)) {
/* the target service is in bad shape, let's avoid it for
* the subsequent requests. And we currently we choose to
Expand Down

0 comments on commit a12a8dd

Please sign in to comment.