jschyma / open_fints_js_client

FinTS/HBCI Javascript Client
Apache License 2.0
134 stars 44 forks source link

Fixes #12 #13

Closed fbrinker closed 8 years ago

fbrinker commented 8 years ago

Added error handling and a callback call

jschyma commented 8 years ago

Zum ersten Teil:

me.bpd.url = bankenlist==undefined?bankenliste[""+in_blz].url:bankenlist[""+in_blz].url;

Ja da hast du recht, das kann man abfangen. Ich habe gerade gesehen, das ist natürlich sowieso Quark dieser Inline IF, wenn ich in beiden Zweigen sowieso dann das gleiche mache. Ist glaub ein Überbleibsel von alten Tests am Anfang der Implementierung. Ich denke wir sollten das aber dann so abfangen:

if(bankenlist){
  if((""+in_blz) in bankenlist && "url" in bankenliste[""+in_blz])
    me.bpd.url = bankenliste[""+in_blz].url;
  else
    throw new Exceptions.WrongInputParameterError("URL Daten für die Bank mit der BLZ "+in_blz+" wurden in der Bankenliste nicht gefunden.");
}else{
   throw new Exceptions.InternalError("Bankenliste ist null!");
}
// und in Class.js dort wo auch die anderen Exceptions sind unten
// eine neue Exception Klasse
Exceptions.WrongInputParameterError = function(msg_txt){
    Exceptions.OpenFinTSClientException.call(this);
};
util.inherits(Exceptions.WrongInputParameterError, Exceptions.OpenFinTSClientException);

Und zum zweiten Teil siehe mein Kommentar im Issue.

fbrinker commented 8 years ago

Hmm Achtung: bankenlist vs bankenliste.

Das inline if hat ein Fallback auf bankenlist, wenn bankenliste nicht übergeben wurde. Dabei wird bankenlist per require mit den testdaten(?) eingebunden.

War mir nicht sicher ob das so bleiben sollte und daher hab ich es so gelassen. Dachte auch erst es ist beides das gleiche. Oder sehe ich das falsch?

jschyma commented 8 years ago

Ah okay das mit der Bankenliste habe ich total übersehen. Ursprünglich war das so gedacht, dass in var bankenliste = require("./bankenliste.js"); einige Banken bereits mit blz und url ausgeliefert werden. Ich habe mich jetzt aber erstmal dagegen entschieden, weil ich mir vorstellen könnte, dass die Banken da empfindlich drauf reagieren wenn ich auf einmal eine Liste mit URLs online stelle, die sonst nur einzeln Abrufbar sind und nacher startet noch irgendwer eine DDOS Attake auf diese URLs aus der Liste. Dem mächte ich lieber vorbeugen. Die URLs sind ja gut über https://www.hbci-zka.de erreichbar. Wir lassen das mit dem Require aber jetzt denke ich mal, habe die eine Variable umbenannt, dann ist die Verwechslungsgefahr nichtmehr so groß.

Ansonsten habe ich noch ein paar weitere Anpassungen gemacht. https://github.com/jschyma/open_fints_js_client/tree/pr/13 Ich hatte versucht 2 Sachen zu implementieren:

Wenn du nochmal drüber schauen möchtest und noch verbesserungsvorschläge hast gerne, ansonsten merge ich den Branch in den master.

fbrinker commented 8 years ago

Nachtrag: Sah soweit ganz gut aus, werde das auch zukünftig noch etwas weiter testen (: