25

I see a lot of tutorials doing something like this:

http.get("...").subscribe(
  success => console.log('hello success'),
  error => console.log('bye error')
);

I don't know how this works, since there aren't any types or anything, however I tried to do that myself and I end up that the request always goes into success, even if I have an error. What is the problem?

Troublemaker:

this.memberService.create(this.currentMember)
      .subscribe(
        success => {
          let mem: Member = success.json() as Member;
          if (this.selectedOrganization) {
            this.addMemberToOrganization(mem);
          } else if (this.selectedServiceProvider) {
            this.addMemberToServiceProvider(mem);
          } else {
            this.toastsService.error("lbl_users_service_provider_and_organization_undefined");
          }
        },
        error => console.log(error)
      );

Create-Method in the memberService:

  create(member: Member): Observable<any> {
    return this.http
      .post(this.RESOURCE_BASE_URL, member)
      .map(response => {
        if (response.status === 200) this.toastsSerivce.success(this.translateService.instant('lbl_users_member_created'));
        return response;
      })
      .catch(error => this.toastsSerivce.error(this.translateService.instant('lbl_users_member_create_failed')));
  }

I even catch the error, but the subscribe part doesn't seem to care. It fails at success.json(), because if there is an error, there is no json. But if there is an error, I want it to call the error =>... instead of the success. Any advice is highly appreciated.

codepleb
  • 10,086
  • 14
  • 69
  • 111
  • in your place i would place check inside "subscribe" callback: if(data.error) – happyZZR1400 Jun 06 '17 at 09:15
  • @happyZZR1400: I could do that, but that doesn't feel like the correct way. Angular has this error-handling built in, but I just don't know how to use it. – codepleb Jun 06 '17 at 09:17
  • When you talk about an error, are you talking about a 4XX/5XX code? Because that's how angular's builtin Http detects errors. – Supamiu Jun 06 '17 at 09:18
  • @Supamiu: At minimum a status code, but usually also an exception-message provided with it, as far as I know. But anyways, I would be glad if it would even recognise that it is an error. – codepleb Jun 06 '17 at 09:21
  • i think you should get something to be returned inside your "catch" like "return Observable.throw(errMsg);" instead of calling toast... – happyZZR1400 Jun 06 '17 at 09:22
  • How are you testing this error at the moment? It seems like your error is returned with a status code 200, so it is handled as a success response and your success callback gets called. 4XX and 5XX status code should trigger error callback, not the success one. Also, catch has to return an observable, because its signature is (error, observable) => Observable. – Supamiu Jun 06 '17 at 09:23
  • @Supamiu Tests? Those are for ppl that dunno wtf they are doing... WAIT! :D Nah at the moment I do not test this. I manually found that out that this part is not doing it's job properly. I get a 404 status code (validated on the console) returned, but it still goes into 'success'. Btw: I'm writing tests. More on the backend side than on the frontside, but there are some basic ones that check if a method calls the correct endpoint. – codepleb Jun 06 '17 at 09:31
  • Try to add return Observable.of(null); in your catch and move it before your map operator. Observable.of(null) will ensure that your data is not handled if you have an error, but the observable chain is not broken. – Supamiu Jun 06 '17 at 09:34

2 Answers2

16

I think the issue is that you are not throwing the error with an Observable.throw(errMsg).

So, you may just use it like this:

.catch((error:any) => Observable.throw(error.json().error || 'Server error'));

In your example:

create(member: Member): Observable<any> {
    return this.http
      .post(this.RESOURCE_BASE_URL, member)
      .map(response => {
        if (response.status === 200) this.toastsSerivce.success(this.translateService.instant('lbl_users_member_created'));
        return response;
      })
      .catch((error:any) => Observable.throw(this.toastsSerivce.error(this.translateService.instant('lbl_users_member_create_failed'))));
  }

But, you could use an error handler, like the one Angular proposes here:

private handleError (error: Response | any) {
    // In a real world app, you might use a remote logging infrastructure
    let errMsg: string;
    if (error instanceof Response) {
      const body = error.json() || '';
      const err = body.error || JSON.stringify(body);
      errMsg = `${error.status} - ${error.statusText || ''} ${err}`;
    } else {
      errMsg = error.message ? error.message : error.toString();
    }
    console.error(errMsg);
    return Observable.throw(errMsg);
  }

And so, your method would look more like this:

create(member: Member): Observable<any> {
    return this.http
      .post(this.RESOURCE_BASE_URL, member)
      .map(response => {
        if (response.status === 200) this.toastsSerivce.success(this.translateService.instant('lbl_users_member_created'));
        return response;
      })
      .catch(this.handleError);
  }

It's actually cleaner and more reusable for other methods that you may create within your service.

I would suggest to use also a response handler, like the one used by Angular's devs: this.extractData.

Obviusly, inside the error handle method you can put your own custom logic, depends on how you want to show or handle the error.

NOTE: I did not test your code nor the code I posted here. But I wanted to show/express the concept. You should throw the error in order to not going into success everytime. How you handle it depends on you and your App.

SrAxi
  • 19,787
  • 11
  • 46
  • 65
  • 1
    This works, I used the first approach. But I am confused now. So `catch` needs to convert the result to an `Observable.throw()` while the success block can just return whatever object it desires? Means: When you subscribe to a HTTP call, you always automatically get a `success` if you do not explicitly do `Observable.throw`? Ok, it is starting to make sense, since you maybe not always want to handle the error in your subscribe box. I would appreciate if you could confirm this or correct my thought-error. Thanks anyways for your help! :) – codepleb Jun 06 '17 at 09:51
  • If you don't throw error then your are not handling the error, therefore your subscription will always go to success, won't detect an error whatsoever. That's how I understand it too. :D – SrAxi Jun 06 '17 at 09:53
  • 2
    @TrudleR I have found this: https://xgrommx.github.io/rx-book/content/getting_started_with_rxjs/creating_and_querying_observable_sequences/error_handling.html I will treasure it myself and read it a couple of times, I found it very useful. If you notice, when the error is not handled or throwed, you will receive data as it is, even when error/fail. – SrAxi Jun 06 '17 at 09:56
4

It works for me:

this.http.post('http://example.com/path/', {sampleData: 'd'}).subscribe(
  res => {alert('ok!');},
  err => {alert(err.error)}
)
yaya
  • 7,675
  • 1
  • 39
  • 38