Ticket #1642 (closed enhancement: fixed)

Opened 6 months ago

Last modified 3 months ago

CommunicationManager exceptions should throw also TerminalError

Reported by: Jani Laakso Owned by: Marko Gronroos
Priority: undefined Milestone: User Interface Library 5.3.0 RC
Component: gwt-adapter-server Version:
Keywords: Cc:
Known Issue description:
Hours estimate: Deadline (dd.mm.yyyy):
Known Issue version (since): Known Issue title:
Hours done: Depends to:
Affects documentation: no
Known Issue workaround:
Affects release notes: yes Contract:

Description

Rationality is that Application developer should be able to catch any exceptions occuring in Toolkit core classes. Currently there exists situations (at least in CommunicationManager?) where exceptions are thrown by Toolkit but there is no way for application developer to see and let alone react to them in any way.

Currently application developer may catch "some Toolkit" errors like this: 

/**
	 * Example how to catch terminal errors and change default behaviour of
	 * error reporting.
	 */
	@Override
	public void terminalError(Terminal.ErrorEvent event) {
		// Print errors always to standard errors streams too
		System.err.println("Terminal error:");
		event.getThrowable().printStackTrace();
		// And let them fall to GUI too (default)
		super.terminalError(event);
	}

Change History

Changed 6 months ago by Jani Laakso

What I'd perhaps do is these changes to CM exception handling.

Add IOException to there, typical situation that occurs when socket is closed abruptly by the client. Another situation is SocketException?, which one is called depends on Application Server that is used. Rationality here is that we may tip developer better what has happened, this is an exception that typically can be ignored without further inspection.

Or

Unify all exceptions to single

New code could be something like this: 

} catch (SocketException e) {
			// Most likely client browser closed socket
			// do not throw lengthy exceptions by default as typically we can
			// ignore these
			System.err
					.println("Warning: SocketException in CommunicationManager."
							+ " Most likely client (browser) closed socket.");
			// but still let application developer to catch these
			application
					.terminalError(new CommunicationManagerErrorImpl(this, e));
		} catch (IOException e) {
			// Most likely client browser closed socket
			// do not throw lengthy exceptions by default as typically we can
			// ignore these
			System.err
					.println("Warning: SocketException in CommunicationManager."
							+ " Most likely client (browser) closed socket.");
			// but still let application developer to catch these
			application
					.terminalError(new CommunicationManagerErrorImpl(this, e));
		} catch (final Throwable e) {
			// let application developer to catch this exception
			application
					.terminalError(new CommunicationManagerErrorImpl(this, e));
			// this exception should always be "logged", needs further
			// inspection
			e.printStackTrace();
			
			// Get rid of this code:
			// Do not tell the client exception, it's an security issue and
			// typically it only breaks UIDL response.
			// Solution here is that now application developer has the control
			// to catch all exceptions and e.g. pass them to client on next XHR
			// if the developer so wishes

			if (false) {
				// Writes the error report to client
				// FIXME breaks UIDL response, security shouldn't reveal stack
				// trace
				// to client side
				final OutputStreamWriter w = new OutputStreamWriter(out);
				final PrintWriter err = new PrintWriter(w);
				err
						.write("<html><head><title>Application Internal Error</title></head><body>");
				err.write("<h1>" + e.toString() + "</h1><pre>\n");
				e.printStackTrace(new PrintWriter(err));
				err.write("\n</pre></body></html>");
				err.close();
			}
		}

About the new proposed "general" CommunicationManager? error event:

  • Well, perhaps we do not want to give reference of CommunicationManager? (error owner) for the application developer, but you get the point..

Changed 6 months ago by Joonas Lehtinen

  • milestone set to IT Mill Sponsored Backlog

Changed 6 months ago by Joonas Lehtinen

  • milestone changed from IT Mill Sponsored Backlog to User Interface Library 5.3.0 RC1

Changed 6 months ago by Jani Laakso

Another typical situation is general exception handler for any toolkti application.

Application developer would like to throw any kind of exceptions (e.g. OperationNotSupportedException? which is defined by Toolkit) and trust that any exceptions are finally handled by Application developer's own exception handler. Currently this is not the case because most exceptions finally are handled by CommunicationManager? BUT it does not pass exceptions to application developers own overridden Application.terminalError as it should.

Changed 3 months ago by Joonas Lehtinen

  • milestone changed from User Interface Library 5.3.0 RC to IT Mill Sponsored Backlog

Changed 3 months ago by Joonas Lehtinen

  • type changed from defect to enhancement

Changed 3 months ago by Jani Laakso

See also #2002 (duplicate?)

Changed 3 months ago by Artur Signell

  • status changed from new to closed
  • resolution set to fixed
  • milestone changed from IT Mill Sponsored Backlog to User Interface Library 5.3.0 RC

Removed all special error handling from CommunicationManager?. It is up to the terminal error handler to decide if IOExceptions and SocketExceptions? can "typically be ignored". The default application error handler ignores SocketExceptions? with a warning text.

Changeset [5242]

Note: See TracTickets for help on using tickets.