Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

API call's error/exception handling #40

Open
mohamnag opened this issue Mar 9, 2018 · 6 comments
Open

API call's error/exception handling #40

mohamnag opened this issue Mar 9, 2018 · 6 comments

Comments

@mohamnag
Copy link

mohamnag commented Mar 9, 2018

I just discovered this lib and it seems very promising, however I could not figure out one piece by looking at the code. That is, I don't get how error responses are handled or handed over to the client.

In my case, the API returns a serialized runtime exception object as JSON in response body when an exception happens in response to a request. The HTTP status code reflects the state of error too. Exceptions are typed (defined classes extending RuntimeException) so the hope was to be able to have their structure also on Angular side and be able to deserialize them on client side just like success responses.

It seems however this is somehow not handled by this lib, am I right?

@komu
Copy link
Member

komu commented Mar 9, 2018

Hi, thanks for your feedback!

You're right, there's no special support at the moment. You can of course call catchError on the returned Observable or pass error-handler to subscribe, but all you get back is the error as returned by Angular's HttpClient.

I've had some thoughts about a higher-level API, but I'm not ready to commit to any particular plan yet. At the moment, I've had a separate ErrorService in my Angular-application for generalized handling of errors. Then my code ends up something like:

constructor(fooEndpoint: FooEndpoint, errorService: ErrorService) {
    fooEndpoint.findFoos().subscribe(foos => {
        console.log("got foos", foos);
    }, e => errorService.showLoadError(e));
}

Alternatively, you can implement your own ApinaEndpointContext and override request handling there:

@NgModule({
    imports: [
        ApinaModule,
    ],
    providers: [
        { provide: ApinaEndpointContext, useClass: MyApinaEndpointContext }
    ]
})
export class MyModule {
}

...

@Injectable()
export class MyApinaEndpointContext extends ApinaEndpointContext {
    ...
}

Or perhaps you could add an interceptor at the level of Angular's HttpClient, since Apina delegates requests to it.

That said, I'm open to suggestions and different ideas.

@komu
Copy link
Member

komu commented Mar 9, 2018

It seems that at the very least Apina could provide two things:

  1. Allow registering some Java-types as needing serialization support even though they are not needed in request/response bodies. So that Apina generates TypeScript types for your exceptions.
  2. Allow easy programmatic access to Apina's serialization mechanism so that you can ask to transform the JSON-response into corresponding class.

After that one could implement an interceptor that would transform the errors semi-automatically.

@mohamnag
Copy link
Author

I also took a look at other, more generic libs which provide Java to TS translation solutions and got following idea:

  1. first of all, as you mentioned, provide a way to include specific classes not used in request/response process to be TS translated. Maybe by setting additional Java package(s)/class name(s)
  2. either provide access to apina's (de)serializer or even better adopt type identification based on specific key values in returned JSON to deserialize to error classes when an error is returned from server. This can be for example type key that Jackson can include in body or anything similar.

I think at least some parts of these match your ideas too.

@mohamnag
Copy link
Author

mohamnag commented Mar 12, 2018

the type identification part could be an optional thing turned on or off during generation. or even better configured (name of key, type of its content and so on) by settings.

@komu
Copy link
Member

komu commented Mar 12, 2018

If Apina would provide a manual API serialiation-api, you could implement second option as easily as you could configure it. Say that you know that every response has an errorCode key. You could then say something like:

// This would be your "configuration"
const errorCodeMappings = {
    "foo": "FooError",
    "bar": "BarError"
}

// Apina would call this callback for all errors before passing them on:
deserializeError(error: Any, serializer: ApinaSerializer) {
    const errorType = errorCodeMappings[error.errorCode];
    if (errorType != null) {
        return serializer.deserialize(error, errorType);
    } else {
        return error;
    }
}

Now the actual configuration (errorCodeMappings) would be as straightforward as specifying it in another place, but the advantage would be that this would be a lot more flexible. Perhaps you'd like to implement this same logic as convention instead of configuration? Just say:

deserializeError(error: Any, serializer: ApinaSerializer) {
    return serializer.deserialize(error, `${capitalize(error.errorCode)}Error`);
}

Or perhaps you need to consult existence of several different fields and perform some custom logic? No problem, since it's all code you could do whatever you need to.

That said, I'm not against a higher level API either, but I think that first it's good to provide the lower level building blocks and get actual experience of different patterns using those. Then it's easier to figure out what the higher level API should be.

As far as the first point goes, I've been thinking about adding optional package of Java-annotations that could be used to configure Apina when defaults are not good enough. Perhaps that would be a nice way to indicate that you want translate some extra classes: add @ApinaTranslated or something like that to your class.

@mohamnag
Copy link
Author

So I will go for class translation first as thats more important to me. I'm actually considering Apina over other solutions as it does not force changes on my backend code like annotations and co. Its just a build step addon that translates what is done and contains all its config in build settings. This was an important criteria for me, otherwise there are libraries that do the Java to TS translation using annotations. If thats also what you intend to at least offer as a possiblity, then it would be great for me too. Otherwise i'm really against having annotations in my backend for this sake.

as for the deserialization, you are right, a custom make deserializer could be assembled by hand, but isn't it the idea here to get as much as possible translated automatically? lets say the "errorCode" key contains the classs name of exception, if I rename the exception class Apina generates the new translated class but the serializer should be updated by hand and in worse case until runtime no hint will be given that the old errorCode does not match any class name which clould be also a long time afterwards. So maybe a kind of plugin functionality would help here to let me write Java/Kotlin code that generates what is needed for error deserialization on build time.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants