Skip to content

Commit

Permalink
Fixed regression with handling probing errors
Browse files Browse the repository at this point in the history
  • Loading branch information
Supereg committed Sep 16, 2020
1 parent 382460c commit bd1424a
Show file tree
Hide file tree
Showing 3 changed files with 23 additions and 14 deletions.
29 changes: 19 additions & 10 deletions src/Responder.ts
Original file line number Diff line number Diff line change
Expand Up @@ -190,15 +190,7 @@ export class Responder implements PacketHandler {

this.promiseChain = this.promiseChain // we synchronize all ongoing probes here
.then(() => service.rebuildServiceRecords()) // build the records the first time for the prober
.then(() => this.probe(service), reason => {
// handle probe error
if (reason === Prober.CANCEL_REASON) {
callback();
} else { // other errors are only thrown when sockets error occur
console.log(`[${service.getFQDN()}] failed probing with reason: ${reason}. Trying again in 2 seconds!`);
return PromiseTimeout(2000).then(() => this.advertiseService(service, callback));
}
});
.then(() => this.probe(service)); // probe errors are catch below

return this.promiseChain.then(() => {
// we are not returning the promise returned by announced here, only PROBING is synchronized
Expand All @@ -211,6 +203,23 @@ export class Responder implements PacketHandler {
});

callback(); // service is considered announced. After the call to the announce() method the service state is set to ANNOUNCING
}, reason => {
/*
* I know seems unintuitive to place the probe error handling below here, miles away from the probe method call.
* Trust me it makes sense (encountered regression now two times in a row).
* 1. We can't put it in the THEN call above, since then errors simply won't be handled from the probe method call.
* (CANCEL error would be passed through and would result in some unwanted stack trace)
* 2. We can't add a catch call above, since otherwise we would silence the CANCEL would be silenced and announce
* would be called anyways.
*/

// handle probe error
if (reason === Prober.CANCEL_REASON) {
callback();
} else { // other errors are only thrown when sockets error occur
console.log(`[${service.getFQDN()}] failed probing with reason: ${reason}. Trying again in 2 seconds!`);
return PromiseTimeout(2000).then(() => this.advertiseService(service, callback));
}
});
}

Expand Down Expand Up @@ -400,7 +409,7 @@ export class Responder implements PacketHandler {

this.server.sendResponseBroadcast( { answers: records }).then(results => {
const failRatio = SendResultFailedRatio(results);
if (failRatio === 1) {
if (failRatio === 1) { // TODO loopback will most likely always succeed
console.log(SendResultFormatError(results, `Failed to send records update for '${service.getFQDN()}'`), true);
if (callback) {
callback(new Error("Updating records failed as of socket errors!"));
Expand Down
4 changes: 2 additions & 2 deletions src/responder/Announcer.ts
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,7 @@ export interface AnnouncerOptions {
*/
export class Announcer {

public static readonly CANCEL_REASON = "cancelled";
public static readonly CANCEL_REASON = "CIAO ANNOUNCEMENT CANCELLED";

private readonly server: MDNSServer;
private readonly service: CiaoService;
Expand Down Expand Up @@ -146,7 +146,7 @@ export class Announcer {

Announcer.sendResponseAddingAddressRecords(this.server, this.service, records, this.goodbye).then(results => {
const failRatio = SendResultFailedRatio(results);
if (failRatio === 1) {
if (failRatio === 1) { // TODO loopback will most likely always succeed
console.error(SendResultFormatError(results, `[${this.service.getFQDN()}] Failed to send ${this.goodbye? "goodbye": "announcement"} requests`), true);
this.promiseReject!(new Error(`${this.goodbye? "Goodbye": "Announcement"} failed as of socket errors!`));
return; // all failed => thus announcement failed
Expand Down
4 changes: 2 additions & 2 deletions src/responder/Prober.ts
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@ const debug = createDebug("ciao:Prober");
*/
export class Prober {

public static readonly CANCEL_REASON = "cancelled";
public static readonly CANCEL_REASON = "CIAO PROBING CANCELLED";

private readonly server: MDNSServer;
private readonly service: CiaoService;
Expand Down Expand Up @@ -162,7 +162,7 @@ export class Prober {
authorities: this.records, // include records we want to announce in authorities to support Simultaneous Probe Tiebreaking (RFC 6762 8.2.)
}).then(results => {
const failRatio = SendResultFailedRatio(results);
if (failRatio === 1) {
if (failRatio === 1) { // TODO loopback will most likely always succeed
console.error(SendResultFormatError(results, `Failed to send probe queries for '${this.service.getFQDN()}'`), true);
this.endProbing(false);
this.promiseReject!(new Error("Probing failed as of socket errors!"));
Expand Down

0 comments on commit bd1424a

Please sign in to comment.