TeamShare API comments Gareth Rees, Ravenbrook Limited, 2000-09-11 1. Introduction Ravenbrook is using the TeamShare API extensively to develop the Perforce/TeamTrack integration. We are developing an extension library that makes (parts of) the TeamShare API available from the scripting language Python. The main component of the integration will be written in Python. You can see the design notes for the TeamTrack Python extension at , and the code at . The memory management and error-handling requirements of a Python extension intended for use in a persistent network service like the Perforce/TeamTrack integration are more stringent than those of a program that runs quickly and exits (like the examples distributed with the TeamShare API). So I have some suggestions for changes to the API sources. 2. Error handling and reporting 2.1. Requirements for error handling and reporting 2.1.1. Errors should be reported whenever they occur. 2.1.2. As much relevant information as practical should be reported about the cause and circumstances of the error. 2.1.3. The documentation should indicate for each function which exceptional values that function can return, the meaning of each exceptional value, and how to reliably get additional information about the error. 2.2. Error handling defects in the TeamShare API 2.2.1. Documentation For most functions, the TeamShare API manual does not indicate which exceptional values that function may return. It does not specify when error messages may be reliably obtained, and when error messages are absent or incorrect. (Requirement 2.1.3.) 2.2.2. Memory management A number of functions call allocation functions (new, malloc, realloc) but don't check the return value. (Requirement 2.1.1.) For example, in TSRecord.C: TSObject *TSRecord::Duplicate() { TSRecord *rec = new TSRecord; rec->tableId = tableId; ... } 2.2.3. Exceptional return but no error code recorded A number of functions return an exceptional value but don't record the error code, so a call to TSGetLastError will return the wrong error code. (Requirement 2.1.3.) For example, in TSServer.C: int TSServer::BuildFieldList(int tableId, TSFieldList &fieldList) { TSSchema *schema = GetSchema(tableId); if (schema == NULL) return TS_ERROR; ... } 2.2.4. Exceptional return but no error message recorded A number of functions set an error code using TSSetLastError, but neither set nor clear the error message. (Requirements 2.1.2, 2.1.3.) For example, in TSServer.C: int TSServer::Connect(const char *userName, ...) { if(password == NULL || userName == NULL) { TSSetLastError(TS_INVALID_USER); return TS_INVALID_USER; } ... } 2.2.5. Information is lost about the cause of an error A number of functions lose information about errors when passing on an error from a called function to the caller. (Requirement 2.1.2.) For example, in TSRecord.C: int TSRecord::Receive(TSSocket *socket) { ... if( socket->ReceiveInt(&sqlDataType) != TS_OK ) { return TS_ERROR; } ... } The error code returned from the ReceiveInt method is lost. 2.2.6. No information recorded about cause of error at TeamShare server When there's an error at the TeamShare server (for example, I might send incorrect SQL as part of a TSServer::ReadRecordListWithWhere method call), no information about the SQL error in the database is returned. (Requirement 2.1.2.) The code looks like this: int TSServer::ReadRecordListWithWhere(...) { ... socket->ReceiveInt(&result); if( !result ) { delete socket; TSSetLastError(TS_ERROR); return TS_ERROR; } ... } I think that the ReceiveInt call sets result to 0 in a variety of circumstances: socket closed, server not responding, error in server, error in database. No information is available to distinguish between these error cases or to help in tracing and fixing the error. 2.2.7. Lack of detailed information about the cause of errors The TeamShare API often fails to report detailed information about the cause of an error. (Requirement 2.1.2.) For example, in TSSocket.C, a code indicating the cause of a socket error is available on Windows as the results of a call to the function WSAGetLastError, and on Unix as the variable errno. It would make sense to include the value of this error code in the error message when a socket error occurs. 2.3. Some suggestions for improvements to error handling 2.3.1. Defects 2.2.3, 2.2.4 and 2.2.5 (error codes and messages) The reason why most functions don't record error messages (defect 2.2.4) is that error messages are stored in a member of the TSServer object. This means that error messages can only be recorded by TSServer methods. (The global error code is stored in a global variable.) I can think of three solutions here: 1. Convert the error handling to use C++ exceptions rather than exceptional return values. 2. Store the error message in a global variable (like the error code) so that it can be written to from anywhere in the API. 3. Store a pointer to a TSServer object inside every other object in the API so that they can store the error message in the server object. (TSRecord object already have such a pointer; the others don't.) Solution 1 is probably not viable if you have a requirement to support people who are already using the API. Solution 2 won't work in applications that have multiple TSServer object in different threads. (But the API doesn't support such applications anyway, on account of its global error code, so this solution is still possible.) Solution 3 would require existing users of the API to re-compile their applications, and it increases the size of each object by one pointer. Once you have chosen a solution that allows all functions to record error messages, I suggest you fix defect 2.2.4 (exceptional return but no error message recorded) by providing a single function that both records an error code and an error message, something like: void TSSetError(int code, TSString& message) { ... } (It won't be possible to use this function to report out-of-memory errors since allocating the buffer for the TSString object will fail, but all other errors can be recorded). Then you can make macros that encapsulate all three operations (set error code, set error message, return exceptional value). This will help to ensure consistency of reporting and returning. For functions that return TS_OK on success, error code on failure, you can use something like: #define TS_RAISE_AND_RETURN_STATUS(code, message) \ do { \ int error_code = (code); /* evaluate only once */ \ TSSetError(error_code, (message)); \ return error_code; \ } while (0) For functions that return an object on success, NULL on failure, you can use something like: #define TS_RAISE_AND_RETURN_NULL(code, message) \ do { \ TSSetError((code), (message)); \ return 0; \ } while (0) To fix defect 2.2.5 (information is lost about the cause of an error), I suggest a macro that checks the value of an expression and returns that value if it is exceptional: #define TS_CHECK_AND_PROPAGATE(expression) \ do { \ int error_code = (expression); \ if (error_code != TS_OK) { \ return error_code; \ } \ } while (0) Note that this macro doesn't have to record the error itself since the error will have been recorded when it happened. 2.3.2. Defect 2.2.2 (memory management) The result of each allocation should be checked, and if the allocation fails then an error code should be set and an exceptional value returned from the function. This will result in a cascade of necessary checks, for example since TSString::GetBuffer allocates memory and returns a pointer to a buffer, it should be changed so that it returns 0 in the event of an allocation failure. This means that the result of all calls to that method need to be checked, and so on. There are a few cases where this strategy won't work. For example, in the TSSocket class, the constructor attempts to allocate memory for the socketBuffer member variable and thus may fail. However, constructors in C++ have no return value and so can only signal failure by throwing an exception, which you may have ruled out as a solution for the reason given in 2.3.1 above. So you need to either (1) move the buffer into the TSSocket object so it is allocated by the "new" operator; or (2) set a flag in the TSSocket object (say, socketBuffer = 0) if allocation fails and check the flag after each TSSocket object is constructed. Note that you probably cannot set an error message when memory allocation fails, since to do so probably involves allocating memory to contain the message. (Though you might write a variant TSSetError that takes a char*, and store a constant error message in a char* variable instead of constructing a TSString object.) For the reason given above, the code shown below (from TSServer.C) is unlikely to work as intended, since errorMsg is a TSString, and the assignment will invoke the TSString::operator= method, which will attempt to allocate. TSSocket *TSServer::OpenSocket() { ... if (socket == NULL) { errorMsg = "Error allocating memory for socket."; TSSetLastError(TS_MEMORY_ERROR); return NULL; } ... } 2.3.3. Defect 2.2.6 (server errors) This depends on the protocol for connecting to the server. If it doesn't return any error information, then the protocol will need to be extended. 3. Memory management responsibilities The TeamShare API documentation doesn't say anything about memory management responsibility. A user of the API needs to know who has responsibility for deleting the contents of container objects. For example, a TSField object of type TS_DATATYPE_RECORDLIST deletes its contents when it is deleted. But a TSRecordList object doesn't delete its contains when it is deleted: you have to call the object's EmptyAndDestroyList method. This difference is not itself important (in fact, it is convenient for the Python extension that a TSRecordList doesn't delete its contents), but the responsibility needs to be documented. 4. Names for values The header file TSDef.h contains many collections of constants: error codes, table ids, field types and so on. In the Python extension it is useful to be able to have names for these constants, for naming them in Python and for reporting errors. At the moment I simply create tables like this: static struct { int code; char *name; } teamtrack_error_codes = { { TS_INVALID_DATA_TYPE, "invalid data type" }, { TS_NO_PERMISSION, "no permission" }, { TS_NO_SUCH_FIELD, "no such field" }, { TS_MEMORY_ERROR, "out of memory" }, ... }; but these tables have to be maintained whenever the TeamShare API changes. It would be better if I could get the names of the constants from the TeamShare API. One approach to this kind of requirement is to have a "relation header" which acts like a table in a relational database. For example, the header "ts-error-code.h" would have content like this: TS_ERROR_CODE(TS_INVALID_DATA_TYPE, -2, "invalid data type") TS_ERROR_CODE(TS_NO_PERMISSION, -3, "no permission") TS_ERROR_CODE(TS_NO_SUCH_FIELD, -4, "no such field") TS_ERROR_CODE(TS_MEMORY_ERROR, -5, "out of memory") ... In TSDef.h you would write: #define TS_ERROR_CODE(code, value, name) const int code = value; #include "ts-error-code.h" and in my Python extension I would write: static struct { int code; char *name; } teamtrack_error_codes = { #define TS_ERROR_CODE(code, value, name) { code, name }, #include "ts-error-code.h" }; There would be one relational header for each relation. The three relations I am using at the moment are the error code relation, the field type relation (that is, the TS_FLDTYPE_* constants) and the table id relation. (The last of these I could get by issuing a query on the TS_TABLES table.) 5. Documentation 5.1. Build instructions The documentation doesn't give much help on getting TSAPI applications to build using Microsoft Visual C++ on Windows. I deduced the following conditions: - Link with tsapiwin32.lib when building an MFC application, tsapi.lib and ws2_32.lib otherwise (specify these in Project -> Settings -> Link -> Object/Library modules). - On non-MFC Windows applications, call TSInitializeWinsock() before trying to open a socket. These are probably worth stating somewhere in the documentation. I get a warning when linking my application: LINK : warning LNK4098: defaultlib "LIBCD" conflicts with use of other libs; use /NODEFAULTLIB:library This doesn't seem to stop the application working. 5.2. Method descriptions The documentation misses out necessary details of the behaviour of the methods. I will give some examples to illustrate the kinds of omissions. The documentation for TSServer::UpdateRecord says: int UpdateRecord(TSRecord *rec, TSRecord *newRecord /*=NULL*/) - Use to update a single record in a table (as opposed to updating a record in a list). Which of the arguments is the record to be updated? For which tables can I use this function? What exceptional values might it return and in what circumstances? What is the role of the newRecord parameter? What happens if I pass the same TSRecord pointer for both arguments? What does it return? The documentation for TSServer::ReadRecordListWithWhere says: int ReadRecordListWithWhere(TSRecordList *list, int tableId, const char *whereClause) - Use to filter a list of records based on the conditions passed; note that the whereClause does not need the word WHERE. For example: "TS_COMPANYID = 5". The table and column names are case-sensitive and must match exactly what is in the database. TeamTrack tables and columns are always UPPERCASE. I have discovered (by experimentation) that I can pass extra bits of SQL as part of the whereClause, for example I can send "TS_ID > 7 ORDER BY TS_ID DESC" and I get the result I expect (the results are in descending order of TS_ID). Is this just an artifact of the implementation or is it a feature I can depend on? The documentation for TSServer::AddField says: int AddField(TSRecord *field, int tableid, int fieldType) - Use to add a field to a given table. Calling this function will perform the AddRecord() on the Fields table and then uses that information to add the field to the appropriate table. Fields can only be added to the following tables: Cases, Incidents, Companies, Contacts, Merchandise, Products, Problems, Resolutions, and Service Agreements. What should I pass as the tableid parameter? (The table I want to add the field to, or TS_TBLID_FIELDS?) What should I pass as the fieldType parameter? (The TS_FLDTYPE_* constant that is in the FLDTYPE field of the first parameter?) The documentation for TSServer::Submit says: int Submit(int *nIssueId, TSString &sLoginid, TSRecord *pRec, int nTableId, int nProjectId, int nFolderId, int nType) - Used to submit an Issue or an Incident into the database and into the workflow. For submitting other items, Companies, Contacts, etc., use AddRecord, since only Issues and Incidents follow a workflow. What are the meanings of the arguments? (I can guess plausible meanings for most of them, but nType is completely obscure.) These are examples only, there are similar omissions throughout. 5.3. Mistake in TSServer::ReadStateList The documentation forTSServer::ReadStateList says: int ReadStateList(TSRecordList *list, int workflowId, BOOL incParent /*=FALSE*/) - Use to obtain a list of states defined for a given workflow; possible to include the parent workflow's states However, it appears that the second argument to ReadStateList needs to be a project id, not a workflow id. (The method is successful when given the id of a project when there is no corresponding workflow with that id, but generates an error in the other case.)